RFR: 8267754: cds/appcds/loaderConstraints/LoaderConstraintsTest.java fails on x86_32 due to customized class loader is not supported [v2]

[Jie Fu]

On Fri, 28 May 2021 07:49:36 GMT, Igor Ignatyev <iignatyev at openjdk.org> wrote:

>>> I think a better solution would be to split the test into two subtests (two separate jtreg test descriptions in one .java file): one for `doTest` another for `doTestCustomLoader` and use `@requires vm.cds.custom.loaders` in the 2nd one. this way, we will get more clear test results.
>>> 
>>> Cheers,
>>> -- Igor
>> 
>> 
>> Thanks @iignatev for your review and sharing.
>> I didn't know this skill before.
>> 
>> Splitting the test into two subtests seems making a tiny bit sense since the CI/CD won't care which sub-tests are not run.
>> 
>> I prefer the current (minimal) change since I think it's already good enough and I didn't find an easy way to rewrite it.
>> 
>> Thanks.
>
> well, smth like[1] should do the trick (haven't tested) and isn't that big of the change.
> 
> -- Igor
> 
> [1]
> ```diff --git a/test/hotspot/jtreg/runtime/cds/appcds/loaderConstraints/LoaderConstraintsTest.java b/test/hotspot/jtreg/runtime/cds/appcds/loaderConstraints/LoaderConstraintsTest.java
> index 9c6d53e7092..5acfc69479a 100644
> --- a/test/hotspot/jtreg/runtime/cds/appcds/loaderConstraints/LoaderConstraintsTest.java
> +++ b/test/hotspot/jtreg/runtime/cds/appcds/loaderConstraints/LoaderConstraintsTest.java
> @@ -22,10 +22,10 @@
>   */
>  
>  /**
> - * @test
> + * @test id=default-cl
>   * @requires vm.cds
>   * @summary Test class loader constraint checks for archived classes
> - * @bug 8267347 8267754
> + * @bug 8267754
>   * @library /test/lib
>   *          /test/hotspot/jtreg/runtime/cds/appcds
>   *          /test/hotspot/jtreg/runtime/cds/appcds/test-classes
> @@ -34,6 +34,20 @@
>   * @run driver LoaderConstraintsTest
>   */
>  
> +/**
> + * @test id=custom-cl
> + * @requires vm.cds.custom.loaders
> + * @summary Test class loader constraint checks for archived classes with custom class loader
> + * @bug 8267347 8267754
> + * @library /test/lib
> + *          /test/hotspot/jtreg/runtime/cds/appcds
> + *          /test/hotspot/jtreg/runtime/cds/appcds/test-classes
> + * @modules java.base/jdk.internal.misc
> + *          jdk.httpserver
> + * @run driver LoaderConstraintsTest custom
> + */
> +
> +
>  import com.sun.net.httpserver.HttpExchange;
>  import com.sun.net.httpserver.HttpHandler;
>  import jdk.test.lib.Asserts;
> @@ -109,8 +123,9 @@ public class LoaderConstraintsTest  {
>  
>      public static void main(String... args) throws Exception {
>          appJar = ClassFileInstaller.writeJar("loader_constraints.jar", appClasses);
> -        doTest();
> -        if (Platform.areCustomLoadersSupportedForCDS()) {
> +        if (args.length == 0) {
> +            doTest();
> +        } else {
>              loaderJar = ClassFileInstaller.writeJar("custom_app_loader.jar", loaderClasses);
>              doTestCustomLoader();
>          }

> well, smth like[1] should do the trick (haven't tested) and isn't that big of the change.
> 
> -- Igor
> 
> [1]
> 
> ```diff
> index 9c6d53e7092..5acfc69479a 100644
> --- a/test/hotspot/jtreg/runtime/cds/appcds/loaderConstraints/LoaderConstraintsTest.java
> +++ b/test/hotspot/jtreg/runtime/cds/appcds/loaderConstraints/LoaderConstraintsTest.java
> @@ -22,10 +22,10 @@
>   */
>  
>  /**
> - * @test
> + * @test id=default-cl
>   * @requires vm.cds
>   * @summary Test class loader constraint checks for archived classes
> - * @bug 8267347 8267754
> + * @bug 8267754
>   * @library /test/lib
>   *          /test/hotspot/jtreg/runtime/cds/appcds
>   *          /test/hotspot/jtreg/runtime/cds/appcds/test-classes
> @@ -34,6 +34,20 @@
>   * @run driver LoaderConstraintsTest
>   */
>  
> +/**
> + * @test id=custom-cl
> + * @requires vm.cds.custom.loaders
> + * @summary Test class loader constraint checks for archived classes with custom class loader
> + * @bug 8267347 8267754
> + * @library /test/lib
> + *          /test/hotspot/jtreg/runtime/cds/appcds
> + *          /test/hotspot/jtreg/runtime/cds/appcds/test-classes
> + * @modules java.base/jdk.internal.misc
> + *          jdk.httpserver
> + * @run driver LoaderConstraintsTest custom
> + */
> +
> +
>  import com.sun.net.httpserver.HttpExchange;
>  import com.sun.net.httpserver.HttpHandler;
>  import jdk.test.lib.Asserts;
> @@ -109,8 +123,9 @@ public class LoaderConstraintsTest  {
>  
>      public static void main(String... args) throws Exception {
>          appJar = ClassFileInstaller.writeJar("loader_constraints.jar", appClasses);
> -        doTest();
> -        if (Platform.areCustomLoadersSupportedForCDS()) {
> +        if (args.length == 0) {
> +            doTest();
> +        } else {
>              loaderJar = ClassFileInstaller.writeJar("custom_app_loader.jar", loaderClasses);
>              doTestCustomLoader();
>          }
> ```


Good.
The patch is much better than what I had thought it might be.
But it's still bigger than the integrated one.

I've learned your skill and will use it next time if necessary.
Thank you so much, @iignatev .

-------------

PR: https://git.openjdk.java.net/jdk/pull/4198