RFR: 8311591: Add SystemModulesPlugin test case that splits module descriptors with new local variables defined by DedupSetBuilder [v4]

[Mandy Chung]

On Tue, 15 Aug 2023 19:13:31 GMT, Christoph <duke at openjdk.org> wrote:

>> Add new test case with sample modules that contains some requires/exports/uses/provides.
>> 
>> We are just unsure if and how we should add some last step of verificaiton with the extracted and decompiled class.
>> 
>> Follow up task from https://github.com/openjdk/jdk/pull/14408
>
> Christoph has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Add another required transitive desktop
>    
>    Assert number of module description generated sub modules
>    
>    Co-authored-by: Oliver Kopp <kopp.dev at gmail.com>
>  - Add copyright header and apply some formatting
>    Revert unnecesary CompilerUtil

Looks good.   Since the image is created with batchSize = 1, it means that one `subX` method is created for each module linked in the image.  So the test can count the number of modules in the image and the number of `subX` methods should match the module count.  I include the suggested code to check that.

test/jdk/tools/jlink/JLinkDedupTestBatchSizeOne.java line 38:

> 36: /*
> 37:  * @test
> 38:  * @summary Make sure that modules can be linked using jlink and deduplication works correctly when creating sub methods

please wrap this long line.

test/jdk/tools/jlink/JLinkDedupTestBatchSizeOne.java line 94:

> 92:                 .addMods("m1")
> 93:                 .addMods("m2")
> 94:                 .addMods("m2")

`m2` is added twice.  The duplicated line should be removed.

test/jdk/tools/jlink/JLinkDedupTestBatchSizeOne.java line 116:

> 114: 
> 115:         extractJImage(image);
> 116:         decompileWitJavap(image);

No needed.   These 2 lines along with the methods can be deleted.

test/jdk/tools/jlink/dedup/src/m1/p1/AInterface.java line 1:

> 1: package p1;

missing copyright header.

test/jdk/tools/jlink/dedup/src/m4/p4/Main.java line 43:

> 41:         }
> 42:         var moduleClass = Class.forName("jdk.internal.module.SystemModules$all");
> 43:         long subMethodCount = Arrays.stream(moduleClass.getDeclaredMethods()).filter(method -> method.getName().startsWith("sub")).count();

nit: wrap long line

Suggestion:

        long subMethodCount = Arrays.stream(moduleClass.getDeclaredMethods())
                                    .filter(method -> method.getName().startsWith("sub"))
                                    .count();

test/jdk/tools/jlink/dedup/src/m4/p4/Main.java line 47:

> 45:         if (subMethodCount != MODULE_SUB_METHOD_COUNT) {
> 46:             throw new AssertionError("Difference in generated sub module methods count! Expected: " + MODULE_SUB_METHOD_COUNT + " but was " + subMethodCount);
> 47:         }

Suggestion:

        // one subX method per each module is generated as the image is linked with
        // --system-modules=batchSize=1
        var moduleCount = ModuleFinder.ofSystem().findAll().stream().count();
        if (subMethodCount != moduleCount) {
            throw new AssertionError("Difference in generated sub module methods count! Expected: " +
                    moduleCount + " but was " + subMethodCount);
        }

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

PR Review: https://git.openjdk.org/jdk/pull/15234#pullrequestreview-1579273111
PR Review Comment: https://git.openjdk.org/jdk/pull/15234#discussion_r1295023900
PR Review Comment: https://git.openjdk.org/jdk/pull/15234#discussion_r1295023665
PR Review Comment: https://git.openjdk.org/jdk/pull/15234#discussion_r1295022781
PR Review Comment: https://git.openjdk.org/jdk/pull/15234#discussion_r1295024085
PR Review Comment: https://git.openjdk.org/jdk/pull/15234#discussion_r1295029909
PR Review Comment: https://git.openjdk.org/jdk/pull/15234#discussion_r1295025060