RFR: 8294977: Convert test/jdk/java tests from ASM library to Classfile API

Alan Bateman alanb at openjdk.org
Tue Mar 14 08:00:09 UTC 2023


On Tue, 14 Mar 2023 02:43:41 GMT, liach <duke at 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


More information about the serviceability-dev mailing list