On Tue, 14 Mar 2023 02:43:41 GMT, liach <d...@openjdk.org> wrote:

> Summaries:
> 1. A few recommendations about updating the constant API is made at 
> https://mail.openjdk.org/pipermail/classfile-api-dev/2023-March/000233.html 
> and I may update this patch shall the API changes be integrated before
> 2. One ASM library-specific test, `LambdaAsm` is removed. Others have their 
> code generation infrastructure upgraded from ASM to Classfile API.
> 3. Most tests are included in tier1, but some are not:
> In `:jdk_io`: (tier2, part 2)
> 
> test/jdk/java/io/Serializable/records/SerialPersistentFieldsTest.java
> test/jdk/java/io/Serializable/records/ProhibitedMethods.java
> test/jdk/java/io/Serializable/records/BadCanonicalCtrTest.java
> 
> In `:jdk_instrument`: (tier 3)
> 
> test/jdk/java/lang/instrument/RetransformAgent.java
> test/jdk/java/lang/instrument/NativeMethodPrefixAgent.java
> test/jdk/java/lang/instrument/asmlib/Instrumentor.java
> 
> 
> @asotona Would you mind reviewing?

Good to see these tests converted, just a few nits about trying to keep the 
code/style consistent with the existing code/style where possible.

test/jdk/java/lang/ModuleTests/AnnotationsTest.java line 146:

> 144:      */
> 145:     static byte[] addDeprecated(byte[] bytes, boolean forRemoval, String 
> since) {
> 146:         return 
> Classfile.parse(bytes).transform(ClassTransform.ACCEPT_ALL.andThen(ClassTransform.endHandler(clb
>  -> {

The conversion of this test okay but would be good if you split up the overly 
long lines as they are inconsistent with everything else in this test and makes 
it annoying to look at the changes side-by-side.

test/jdk/java/lang/invoke/defineHiddenClass/BasicTest.java line 282:

> 280: 
> 281:         assertTrue(hc.isHidden());
> 282:         assertEquals(hc.getModifiers(), 
> accessFlags.stream().mapToInt(AccessFlag::mask).reduce(AccessFlag.PUBLIC.mask(),
>  (a, b) -> a | b));

Do you mind splitting this line too, it's 140+ characters long so impossible to 
look at the changes side-by-side.

test/jdk/java/util/ServiceLoader/BadProvidersTest.java line 216:

> 214:             clb.withSuperclass(CD_Object);
> 215:             clb.withFlags(AccessFlag.PUBLIC, AccessFlag.SUPER);
> 216:             var provider$1Desc = ClassDesc.of("p", "ProviderFactory$1");

This is class descriptor for ProviderFactory$1, not "Provider" so maybe rename 
this to providerFactory1 or something a bit clearer.

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

PR: https://git.openjdk.org/jdk/pull/13009

Reply via email to