On Wed, 7 Jun 2023 16:06:54 GMT, Mandy Chung <mch...@openjdk.org> wrote:
> `FieldSetAccessibleTest` attempts to load all JDK classes and test > `setAccessible` on their public fields to work properly. It should filter > the modules that directly and indirectly depend on `jdk.internal.vm.compiler` > and `jdk.internal.vm.compiler.management` as they are upgradeable and also > provide APIs to add qualifed exports dynamically. LGTM. I have suggested adding a comment to explain what's going on in the code added. test/jdk/java/lang/Class/getDeclaredField/FieldSetAccessibleTest.java line 323: > 321: mods.stream() > 322: .flatMap(mn -> > findDeps(mn, inverseDeps).stream())) > 323: .collect(Collectors.toSet()); Suggestion: // Build a list of modules that should be filtered out. These are the deploy modules, // plus all modules that directly or indirectly require jdk.internal.vm.compiler and // jdk.internal.vm.compiler.management, as these are upgradeable and also provide // APIs to add qualified exports dynamically. Set<String> filters = Stream.concat(deployModules.stream(), mods.stream() .flatMap(mn -> findDeps(mn, inverseDeps).stream())) .collect(Collectors.toSet()); ------------- Marked as reviewed by dfuchs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14362#pullrequestreview-1468243596 PR Review Comment: https://git.openjdk.org/jdk/pull/14362#discussion_r1221957008