[jdk17u-dev] RFR: 8336942: Improve test coverage for class loading elements with annotations of different retentions [v2]

[Thomas Fitzsimmons]

On Sat, 9 Nov 2024 01:08:47 GMT, Liam Miller-Cushon <cushon at openjdk.org> wrote:

>> _I am not a Reviewer; I maintain `java-17-openjdk` in `RHEL` so I try to review `rfr`-labelled pull requests for `jdk17u-dev`, time-permitting._
>> 
>> This looks like a good change to have in 17.  I tested it on `Fedora 41 x86_64`.
>> 
>> While trying this out, I noticed a potential oddity regarding `BasicAnnoTests.java`: the first of the two `Test` annotations does not seem to take effect.  For example, I would expect that changing:
>> 
>> `@Test(posn=0, annoType=TA.class, expect="1")`
>> 
>> to:
>> 
>> `@Test(posn=0, annoType=TA.class, expect="33")`
>> 
>> would cause a failure, but it does not.  Am I missing something?
>> 
>> I am testing with jtreg `857ed6167418cb4ebe2844fd536461a1649bdced` running on `jdk-17.0.13+11`.
>
> @fitzsim I tried with jtreg `857ed6167418cb4ebe2844fd536461a1649bdced` and `17.0.13+11` and I can reproduce a test failure with that change. Could you double-check you're still unable to reproduce that failure?
> 
> 
> diff --git a/test/langtools/tools/javac/processing/model/type/BasicAnnoTests.java b/test/langtools/tools/javac/processing/model/type/BasicAnnoTests.java
> index d4540107022..e22c3d132b9 100644
> --- a/test/langtools/tools/javac/processing/model/type/BasicAnnoTests.java
> +++ b/test/langtools/tools/javac/processing/model/type/BasicAnnoTests.java
> @@ -423,7 +423,7 @@ public class BasicAnnoTests extends JavacTestingAbstractProcessor {
>      //             vary position of one annotated
>      //       the three above with the corner case of the ambiguos decl + type anno added
> 
> -    @Test(posn=0, annoType=TA.class, expect="1")
> +    @Test(posn=0, annoType=TA.class, expect="33")
>      public @TA(1) int f1;
> 
>      @Test(posn=0, annoType=TA.class, expect="11")
> 
> 
> 
> make test TEST="jtreg:test/langtools/tools/javac/processing/model/type/BasicAnnoTests.java"
> ...
> ./src/jdk17u-dev/test/langtools/tools/javac/processing/model/type/BasicAnnoTests.java:427: error: Unexpected value: 1, expected: 33
>     public @TA(1) int f1;
>                       ^
> 1 error

@cushon I should have included a patch for context; I was modifying the first of the two `Test` annotations _added by this pull request_.  Can you try this patch, on top of this pull request?


diff --git a/test/langtools/tools/javac/processing/model/type/BasicAnnoTests.java b/test/langtools/tools/javac/processing/model/type/BasicAnnoTests.java
index d4540107022..06bd32bcfbb 100644
--- a/test/langtools/tools/javac/processing/model/type/BasicAnnoTests.java
+++ b/test/langtools/tools/javac/processing/model/type/BasicAnnoTests.java
@@ -535,7 +535,7 @@ public class BasicAnnoTests extends JavacTestingAbstractProcessor {
     @Test(posn=1, annoType=TA.class, expect="23")
     public Set<@TA(23) ? super Object> f9;
 
-    @Test(posn=0, annoType=TA.class, expect="1")
+    @Test(posn=0, annoType=TA.class, expect="33")
     @Test(posn=0, annoType=TD.class, expect="2")
     public @TA(1) @TD(2) int f10;
 

I get:


make test TEST="jtreg:test/langtools/tools/javac/processing/model/type/BasicAnnoTests.java"
...
Passed: tools/javac/processing/model/type/BasicAnnoTests.java
Test results: passed: 1
...
TEST SUCCESS

but I would expect a failure.

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

PR Comment: https://git.openjdk.org/jdk17u-dev/pull/2955#issuecomment-2466116287