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