8320716: ResolvedModule::reads includes self when configuration contains two or more automatic modules

Reviewed-by: mchung

src/java.base/share/classes/java/lang/module/ResolvedModule.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -97,6 +97,8 @@ public final class ResolvedModule {
 
     /**
      * Returns the set of resolved modules that this resolved module reads.
+     * The readability relation is reflexive (every module reads itself). The
+     * set of resolved modules returned by this method does not include itself.
      *
      * @return A possibly-empty unmodifiable set of resolved modules that
      *         this resolved module reads

src/java.base/share/classes/java/lang/module/Resolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -497,7 +497,6 @@ final class Resolver {
      * and m2 reads m3.
      */
     private Map<ResolvedModule, Set<ResolvedModule>> makeGraph(Configuration cf) {
-
         int moduleCount = nameToReference.size();
 
         // the "reads" graph starts as a module dependence graph and
@@ -537,9 +536,7 @@ final class Resolver {
         }
 
         // populate g1 and g2 with the dependences from the selected modules
-
         Map<String, ResolvedModule> nameToResolved = HashMap.newHashMap(moduleCount);
-
         for (ModuleReference mref : nameToReference.values()) {
             ModuleDescriptor descriptor = mref.descriptor();
             String name = descriptor.name();
@@ -595,8 +592,7 @@ final class Resolver {
                     String name2 = descriptor2.name();
 
                     if (!name.equals(name2)) {
-                        ResolvedModule m2
+                        ResolvedModule m2 = computeIfAbsent(nameToResolved, name2, cf, mref2);
-                            = computeIfAbsent(nameToResolved, name2, cf, mref2);
                         reads.add(m2);
                         if (descriptor2.isAutomatic())
                             requiresTransitive.add(m2);
@@ -622,29 +618,33 @@ final class Resolver {
             g2.put(m1, requiresTransitive);
         }
 
-        // Iteratively update g1 until there are no more requires transitive
+        // Iteratively update g1 until there are no more requires transitive to propagate
-        // to propagate
         boolean changed;
         List<ResolvedModule> toAdd = new ArrayList<>();
         do {
             changed = false;
-            for (Set<ResolvedModule> m1Reads : g1.values()) {
+            for (Map.Entry<ResolvedModule, Set<ResolvedModule>> e : g1.entrySet()) {
-                for (ResolvedModule m2 : m1Reads) {
+                ResolvedModule m1 = e.getKey();
-                    Set<ResolvedModule> m2RequiresTransitive = g2.get(m2);
+                // automatic module already reads all selected modules so nothing to propagate
-                    if (m2RequiresTransitive != null) {
+                if (!m1.descriptor().isAutomatic()) {
-                        for (ResolvedModule m3 : m2RequiresTransitive) {
+                    Set<ResolvedModule> m1Reads = e.getValue();
-                            if (!m1Reads.contains(m3)) {
+                    for (ResolvedModule m2 : m1Reads) {
-                                // m1 reads m2, m2 requires transitive m3
+                        Set<ResolvedModule> m2RequiresTransitive = g2.get(m2);
-                                // => need to add m1 reads m3
+                        if (m2RequiresTransitive != null) {
-                                toAdd.add(m3);
+                            for (ResolvedModule m3 : m2RequiresTransitive) {
+                                if (!m1Reads.contains(m3)) {
+                                    // m1 reads m2, m2 requires transitive m3
+                                    // => need to add m1 reads m3
+                                    toAdd.add(m3);
+                                }
                             }
                         }
                     }
-                }
+                    if (!toAdd.isEmpty()) {
-                if (!toAdd.isEmpty()) {
+                        m1Reads.addAll(toAdd);
-                    m1Reads.addAll(toAdd);
+                        toAdd.clear();
-                    toAdd.clear();
+                        changed = true;
-                    changed = true;
+                    }
                 }
             }
         } while (changed);

test/jdk/java/lang/module/AutomaticModulesTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,12 +23,12 @@
 
 /**
  * @test
- * @bug 8142968 8253751 8262944
+ * @bug 8142968 8253751 8262944 8314986
  * @library /test/lib
  * @build AutomaticModulesTest
  *        jdk.test.lib.util.JarUtils
  *        jdk.test.lib.util.ModuleUtils
- * @run testng AutomaticModulesTest
+ * @run junit AutomaticModulesTest
  * @summary Basic tests for automatic modules
  */
 
@@ -54,75 +54,69 @@ import java.util.stream.Stream;
 import jdk.test.lib.util.JarUtils;
 import jdk.test.lib.util.ModuleUtils;
 
-import org.testng.annotations.DataProvider;
+import org.junit.jupiter.api.Test;
-import org.testng.annotations.Test;
+import org.junit.jupiter.params.ParameterizedTest;
-import static org.testng.Assert.*;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
 
-@Test
 public class AutomaticModulesTest {
-
     private static final Path USER_DIR = Path.of(System.getProperty("user.dir"));
 
-    @DataProvider(name = "jarnames")
+    // JAR file name to expected module name / version
-    public Object[][] createJarNames() {
+    static Stream<Arguments> jarNames() {
-        return new Object[][] {
+        return Stream.of(
+            arguments( "foo.jar",                    "foo" ),
+            arguments( "foo4j.jar",                  "foo4j" ),
 
-            // JAR file name                module-name[/version]
+            arguments( "foo1.jar",                   "foo1" ),
+            arguments( "foo10.jar",                  "foo10" ),
 
-            { "foo.jar",                    "foo" },
+            arguments( "foo-1.jar",                  "foo/1" ),
-            { "foo4j.jar",                  "foo4j", },
+            arguments( "foo-1.2.jar",                "foo/1.2" ),
+            arguments( "foo-1.2.3.jar",              "foo/1.2.3" ),
+            arguments( "foo-1.2.3.4.jar",            "foo/1.2.3.4" ),
 
-            { "foo1.jar",                   "foo1" },
+            arguments( "foo-10.jar",                 "foo/10" ),
-            { "foo10.jar",                  "foo10" },
+            arguments( "foo-10.20.jar",              "foo/10.20" ),
+            arguments( "foo-10.20.30.jar",           "foo/10.20.30" ),
+            arguments( "foo-10.20.30.40.jar",        "foo/10.20.30.40" ),
 
-            { "foo-1.jar",                  "foo/1" },
+            arguments( "foo-bar.jar",                "foo.bar" ),
-            { "foo-1.2.jar",                "foo/1.2" },
+            arguments( "foo-bar-1.jar",              "foo.bar/1" ),
-            { "foo-1.2.3.jar",              "foo/1.2.3" },
+            arguments( "foo-bar-1.2.jar",            "foo.bar/1.2" ),
-            { "foo-1.2.3.4.jar",            "foo/1.2.3.4" },
+            arguments( "foo-bar-10.jar",             "foo.bar/10" ),
+            arguments( "foo-bar-10.20.jar",          "foo.bar/10.20" ),
 
-            { "foo-10.jar",                 "foo/10" },
+            arguments("foo.bar1.jar",                "foo.bar1" ),
-            { "foo-10.20.jar",              "foo/10.20" },
+            arguments( "foo.bar10.jar",              "foo.bar10" ),
-            { "foo-10.20.30.jar",           "foo/10.20.30" },
-            { "foo-10.20.30.40.jar",        "foo/10.20.30.40" },
 
-            { "foo-bar.jar",                "foo.bar" },
+            arguments( "foo-1.2-SNAPSHOT.jar",       "foo/1.2-SNAPSHOT" ),
-            { "foo-bar-1.jar",              "foo.bar/1" },
+            arguments( "foo-bar-1.2-SNAPSHOT.jar",   "foo.bar/1.2-SNAPSHOT" ),
-            { "foo-bar-1.2.jar",            "foo.bar/1.2"},
-            { "foo-bar-10.jar",             "foo.bar/10" },
-            { "foo-bar-10.20.jar",          "foo.bar/10.20" },
 
-            { "foo.bar1.jar",               "foo.bar1" },
+            arguments( "foo--bar-1.0.jar",           "foo.bar/1.0" ),
-            { "foo.bar10.jar",              "foo.bar10" },
+            arguments( "-foo-bar-1.0.jar",           "foo.bar/1.0" ),
-
+            arguments( "foo-bar--1.0.jar",           "foo.bar/1.0" )
-            { "foo-1.2-SNAPSHOT.jar",       "foo/1.2-SNAPSHOT" },
+        );
-            { "foo-bar-1.2-SNAPSHOT.jar",   "foo.bar/1.2-SNAPSHOT" },
-
-            { "foo--bar-1.0.jar",           "foo.bar/1.0" },
-            { "-foo-bar-1.0.jar",           "foo.bar/1.0" },
-            { "foo-bar--1.0.jar",           "foo.bar/1.0" },
-
-        };
     }
 
     // JAR file names that do not map to a legal module name
-    @DataProvider(name = "badjarnames")
+    static Stream<String> barJarNames() {
-    public Object[][] createBadNames() {
+        return Stream.of(
-        return new Object[][]{
+            ".jar",
-
+            "_.jar",
-            { ".jar",          null },
+            "foo.1.jar",
-            { "_.jar",         null },
+            "1foo.jar",
-
+            "foo.1bar.jar"
-            { "foo.1.jar",     null },
+        );
-            { "1foo.jar",      null },
-            { "foo.1bar.jar",  null },
-
-        };
     }
 
     /**
      * Test mapping of JAR file names to module names
      */
-    @Test(dataProvider = "jarnames")
+    @ParameterizedTest
+    @MethodSource("jarNames")
     public void testNames(String fn, String mid) throws IOException {
         String[] s = mid.split("/");
         String mn = s[0];
@@ -143,19 +137,20 @@ public class AutomaticModulesTest {
 
         ModuleDescriptor descriptor = mref.get().descriptor();
         assertTrue(descriptor.isAutomatic());
-        assertEquals(descriptor.name(), mn);
+        assertEquals(mn, descriptor.name());
         if (vs == null) {
             assertFalse(descriptor.version().isPresent());
         } else {
-            assertEquals(descriptor.version().get().toString(), vs);
+            assertEquals(vs, descriptor.version().get().toString());
         }
     }
 
     /**
      * Test impossible mapping of JAR files to modules names
      */
-    @Test(dataProvider = "badjarnames", expectedExceptions = FindException.class)
+    @ParameterizedTest
-    public void testBadNames(String fn, String ignore) throws IOException {
+    @MethodSource("barJarNames")
+    public void testBadNames(String fn) throws IOException {
         Path dir = Files.createTempDirectory(USER_DIR, "mods");
         Path jf = dir.resolve(fn);
 
@@ -163,37 +158,36 @@ public class AutomaticModulesTest {
         createDummyJarFile(jf);
 
         // should throw FindException
-        ModuleFinder.of(dir).findAll();
+        var finder = ModuleFinder.of(dir);
+        assertThrows(FindException.class, finder::findAll);
     }
 
-    @DataProvider(name = "modulenames")
+    // Automatic-Module-Name attribute values and version in JAR file name
-    public Object[][] createModuleNames() {
+    static Stream<Arguments> automaticModuleNames() {
-        return new Object[][] {
+        return Stream.of(
-            { "foo",        null },
+            arguments( "foo",        null ),
-            { "foo",        "1.0" },
+            arguments( "foo",        "1.0" ),
-            { "foo.bar",    null },
+            arguments( "foo.bar",    null ),
-            { "foo.bar",    "1.0" },
+            arguments( "foo.bar",    "1.0" ),
-            { "class_",     null },
+            arguments( "class_",     null ),
-            { "class_",     "1.0" },
+            arguments( "class_",     "1.0" )
-        };
+        );
     }
 
-    @DataProvider(name = "badmodulenames")
+    // Bad Automatic-Module-Name attribute values
-    public Object[][] createBadModuleNames() {
+    static Stream<String> badAutomaticModuleNames() {
-        return new Object[][] {
+        return Stream.of(
-            { "",            null },
+            "",
-            { "",            "1.0" },
+            "666",
-            { "666",         null },
+            "foo.class"
-            { "666",         "1.0" },
+        );
-            { "foo.class",   null },
-            { "foo.class",   "1.0" },
-        };
     }
 
     /**
      * Test JAR files with the Automatic-Module-Name attribute
      */
-    @Test(dataProvider = "modulenames")
+    @ParameterizedTest
+    @MethodSource("automaticModuleNames")
     public void testAutomaticModuleNameAttribute(String name, String vs)
         throws IOException
     {
@@ -218,27 +212,26 @@ public class AutomaticModulesTest {
 
         ModuleReference mref = finder.find(name).get();
         ModuleDescriptor descriptor = mref.descriptor();
-        assertEquals(descriptor.name(), name);
+        assertEquals(name, descriptor.name());
-        assertEquals(descriptor.version()
+        assertEquals(vs, descriptor.version()
                 .map(ModuleDescriptor.Version::toString)
-                .orElse(null), vs);
+                .orElse(null));
     }
 
     /**
      * Test JAR files with the Automatic-Module-Name attribute with a value
      * that is not a legal module name.
      */
-    @Test(dataProvider = "badmodulenames", expectedExceptions = FindException.class)
+    @ParameterizedTest
-    public void testBadAutomaticModuleNameAttribute(String name, String ignore)
+    @MethodSource("badAutomaticModuleNames")
-        throws IOException
+    public void testBadAutomaticModuleNameAttribute(String name) throws IOException {
-    {
+        assertThrows(FindException.class, () -> testAutomaticModuleNameAttribute(name, null));
-        // should throw FindException
-        testAutomaticModuleNameAttribute(name, null);
     }
 
     /**
      * Test all packages are exported
      */
+    @Test
     public void testPackages() throws IOException {
         Path dir = Files.createTempDirectory(USER_DIR, "mods");
         createDummyJarFile(dir.resolve("m.jar"),
@@ -263,6 +256,7 @@ public class AutomaticModulesTest {
      * Test class files in JAR file where the entry does not correspond to a
      * legal package name.
      */
+    @Test
     public void testBadPackage() throws IOException {
         Path dir = Files.createTempDirectory(USER_DIR, "mods");
         createDummyJarFile(dir.resolve("m.jar"), "p/C1.class", "p-/C2.class");
@@ -284,6 +278,7 @@ public class AutomaticModulesTest {
     /**
      * Test non-class resources in a JAR file.
      */
+    @Test
     public void testNonClassResources() throws IOException {
         Path dir = Files.createTempDirectory(USER_DIR, "mods");
         createDummyJarFile(dir.resolve("m.jar"),
@@ -307,17 +302,18 @@ public class AutomaticModulesTest {
     /**
      * Test .class file in unnamed package (top-level directory)
      */
-    @Test(expectedExceptions = FindException.class)
+    @Test
     public void testClassInUnnamedPackage() throws IOException {
         Path dir = Files.createTempDirectory(USER_DIR, "mods");
         createDummyJarFile(dir.resolve("m.jar"), "Mojo.class");
-        ModuleFinder finder = ModuleFinder.of(dir);
+        var finder = ModuleFinder.of(dir);
-        finder.findAll();
+        assertThrows(FindException.class, finder::findAll);
     }
 
     /**
      * Test JAR file with META-INF/services configuration file
      */
+    @Test
     public void testServicesConfiguration() throws IOException {
         String service = "p.S";
         String provider = "p.S1";
@@ -345,27 +341,25 @@ public class AutomaticModulesTest {
         ModuleDescriptor descriptor = mref.get().descriptor();
         assertTrue(descriptor.provides().size() == 1);
         ModuleDescriptor.Provides provides = descriptor.provides().iterator().next();
-        assertEquals(provides.service(), service);
+        assertEquals(service, provides.service());
         assertTrue(provides.providers().size() == 1);
         assertTrue(provides.providers().contains((provider)));
     }
 
     // META-INF/services files that don't map to legal service names
-    @DataProvider(name = "badservices")
+    static Stream<Arguments> badServices() {
-    public Object[][] createBadServices() {
+        return Stream.of(
-        return new Object[][] {
+            arguments( "-",  "p.S1" ),
-
+            arguments( ".S", "p.S1" )
-                // service type         provider type
+        );
-                { "-",                  "p.S1" },
-                { ".S",                 "p.S1" },
-        };
     }
 
     /**
      * Test JAR file with META-INF/services configuration file with bad
      * values or names.
      */
-    @Test(dataProvider = "badservices")
+    @ParameterizedTest
+    @MethodSource("badServices")
     public void testBadServicesNames(String service, String provider)
         throws IOException
     {
@@ -383,22 +377,20 @@ public class AutomaticModulesTest {
     }
 
     // META-INF/services configuration file entries that are not legal
-    @DataProvider(name = "badproviders")
+    static Stream<Arguments> badProviders() {
-    public Object[][] createBadProviders() {
+        return Stream.of(
-        return new Object[][] {
+            arguments("p.S", "-"),
-
+            arguments("p.S", "p..S1"),
-                // service type         provider type
+            arguments("p.S", "S1.")
-                { "p.S",                "-" },
+        );
-                { "p.S",                "p..S1" },
-                { "p.S",                "S1." },
-        };
     }
 
     /**
      * Test JAR file with META-INF/services configuration file with bad
      * values or names.
      */
-    @Test(dataProvider = "badproviders", expectedExceptions = FindException.class)
+    @ParameterizedTest
+    @MethodSource("badProviders")
     public void testBadProviderNames(String service, String provider)
         throws IOException
     {
@@ -418,14 +410,15 @@ public class AutomaticModulesTest {
         JarUtils.createJarFile(dir.resolve("m.jar"), tmpdir);
 
         // should throw FindException
-        ModuleFinder.of(dir).findAll();
+        var finder = ModuleFinder.of(dir);
+        assertThrows(FindException.class, finder::findAll);
     }
 
     /**
      * Test JAR file with META-INF/services configuration file listing a
      * provider that is not in the module.
      */
-    @Test(expectedExceptions = FindException.class)
+    @Test
     public void testMissingProviderPackage() throws IOException {
         Path tmpdir = Files.createTempDirectory(USER_DIR, "tmp");
 
@@ -438,31 +431,20 @@ public class AutomaticModulesTest {
         Path jarfile = dir.resolve("m.jar");
         JarUtils.createJarFile(jarfile, tmpdir);
 
-        // catch FindException, inspect its cause's type and details, and rethrow
+        // FindException should be thrown with InvalidModuleDescriptorException as cause
-        var expectedMessage = "Provider class q.P not in JAR file " + jarfile.getFileName();
+        var finder = ModuleFinder.of(dir);
-        try {
+        FindException ex = assertThrows(FindException.class, () -> finder.findAll());
-            ModuleFinder.of(dir).findAll();
+        Throwable cause = ex.getCause();
-        } catch (FindException exception) {
+        assertTrue(cause instanceof InvalidModuleDescriptorException);
-            if (exception.getCause() instanceof InvalidModuleDescriptorException imde) {
+        var imde = (InvalidModuleDescriptorException) cause;
-                var actualMessage = imde.getMessage();
+        assertEquals("Provider class q.P not in JAR file " + jarfile.getFileName(), imde.getMessage());
-                if (actualMessage.equals(expectedMessage)) {
-                    throw exception; // rethrow as expected
-                }
-                throw new AssertionError(
-                    """
-                    Unexpected detail message in InvalidModuleDescriptorException:
-                      Expected message -> '%s'
-                        Actual message -> '%s'
-                    """.formatted(expectedMessage, actualMessage));
-            }
-            throw new AssertionError("Unexpected exception cause: " + exception.getCause());
-        }
     }
 
     /**
      * Test that a JAR file with a Main-Class attribute results
      * in a module with a main class.
      */
+    @Test
     public void testMainClass() throws IOException {
         String mainClass = "p.Main";
 
@@ -481,27 +463,24 @@ public class AutomaticModulesTest {
         Configuration cf = resolve(parent, finder, "m");
 
         ModuleDescriptor descriptor = findDescriptor(cf, "m");
-
+        assertEquals(mainClass, descriptor.mainClass().get());
-        assertTrue(descriptor.mainClass().isPresent());
-        assertEquals(descriptor.mainClass().get(), mainClass);
     }
 
-    // Main-Class files that do not map to a legal qualified type name
+    // Main-Class attribute values that do not map to a legal qualified type name
-    @DataProvider(name = "badmainclass")
+    static Stream<String> badMainClasses() {
-    public Object[][] createBadMainClass() {
+        return Stream.of(
-        return new Object[][] {
+            "p..Main",
-            { "p..Main",     null },
+            "p-.Main"
-            { "p-.Main",     null },
+        );
-
-        };
     }
 
     /**
      * Test that a JAR file with a Main-Class attribute that is not a qualified
      * type name.
      */
-    @Test(dataProvider = "badmainclass")
+    @ParameterizedTest
-    public void testBadMainClass(String mainClass, String ignore) throws IOException {
+    @MethodSource("badMainClasses")
+    public void testBadMainClass(String mainClass) throws IOException {
         Manifest man = new Manifest();
         Attributes attrs = man.getMainAttributes();
         attrs.put(Attributes.Name.MANIFEST_VERSION, "1.0.0");
@@ -521,6 +500,7 @@ public class AutomaticModulesTest {
     /**
      * Test that a JAR file with a Main-Class attribute that is not in the module
      */
+    @Test
     public void testMissingMainClassPackage() throws IOException {
         Manifest man = new Manifest();
         Attributes attrs = man.getMainAttributes();
@@ -544,6 +524,7 @@ public class AutomaticModulesTest {
      *   b*
      *   c*
      */
+    @Test
     public void testConfiguration1() throws Exception {
         ModuleDescriptor descriptor1
             = ModuleDescriptor.newModule("a")
@@ -588,13 +569,14 @@ public class AutomaticModulesTest {
         assertTrue(a.reads().contains(c));
 
         assertTrue(b.reads().contains(a));
+        assertFalse(b.reads().contains(b));
         assertTrue(b.reads().contains(c));
         testReadAllBootModules(cf, "b");  // b reads all modules in boot layer
 
         assertTrue(c.reads().contains(a));
         assertTrue(c.reads().contains(b));
+        assertFalse(c.reads().contains(c));
         testReadAllBootModules(cf, "c");  // c reads all modules in boot layer
-
     }
 
     /**
@@ -604,6 +586,7 @@ public class AutomaticModulesTest {
      *   c*
      *   d*
      */
+    @Test
     public void testInConfiguration2() throws IOException {
         ModuleDescriptor descriptor1
             = ModuleDescriptor.newModule("a")
@@ -660,12 +643,14 @@ public class AutomaticModulesTest {
 
         assertTrue(c.reads().contains(a));
         assertTrue(c.reads().contains(b));
+        assertFalse(c.reads().contains(c));
         assertTrue(c.reads().contains(d));
         testReadAllBootModules(cf, "c");   // c reads all modules in boot layer
 
         assertTrue(d.reads().contains(a));
         assertTrue(d.reads().contains(b));
         assertTrue(d.reads().contains(c));
+        assertFalse(d.reads().contains(d));
         testReadAllBootModules(cf, "d");    // d reads all modules in boot layer
     }
 
@@ -676,6 +661,7 @@ public class AutomaticModulesTest {
      *   c*
      *   d*
      */
+    @Test
     public void testInConfiguration3() throws IOException {
         ModuleDescriptor descriptor1
             = ModuleDescriptor.newModule("a")
@@ -732,18 +718,16 @@ public class AutomaticModulesTest {
         assertTrue(b.reads().contains(d));
         assertTrue(b.reads().contains(base));
 
-        assertTrue(reads(cf, "b", "c"));
-        assertTrue(reads(cf, "b", "d"));
-        assertTrue(reads(cf, "b", "java.base"));
-
         assertTrue(c.reads().contains(a));
         assertTrue(c.reads().contains(b));
+        assertFalse(c.reads().contains(c));
         assertTrue(c.reads().contains(d));
         testReadAllBootModules(cf, "c");   // c reads all modules in boot layer
 
         assertTrue(d.reads().contains(a));
         assertTrue(d.reads().contains(b));
         assertTrue(d.reads().contains(c));
+        assertFalse(d.reads().contains(d));
         testReadAllBootModules(cf, "d");    // d reads all modules in boot layer
     }
 
@@ -751,6 +735,7 @@ public class AutomaticModulesTest {
      * Basic test to ensure that no automatic modules are resolved when
      * an automatic module is not a root or required by other modules.
      */
+    @Test
     public void testInConfiguration4() throws IOException {
         ModuleDescriptor descriptor1
             = ModuleDescriptor.newModule("m1")
@@ -780,6 +765,7 @@ public class AutomaticModulesTest {
      * Basic test to ensure that if an automatic module is resolved then
      * all observable automatic modules are resolved.
      */
+    @Test
     public void testInConfiguration5() throws IOException {
         // m1 requires m2
         ModuleDescriptor descriptor1
@@ -836,6 +822,7 @@ public class AutomaticModulesTest {
 
         assertTrue(auto1.reads().contains(m1));
         assertTrue(auto1.reads().contains(m2));
+        assertFalse(auto1.reads().contains(auto1));
         assertTrue(auto1.reads().contains(auto2));
         assertTrue(auto1.reads().contains(auto3));
         assertTrue(auto1.reads().contains(base));
@@ -843,6 +830,7 @@ public class AutomaticModulesTest {
         assertTrue(auto2.reads().contains(m1));
         assertTrue(auto2.reads().contains(m2));
         assertTrue(auto2.reads().contains(auto1));
+        assertFalse(auto2.reads().contains(auto2));
         assertTrue(auto2.reads().contains(auto3));
         assertTrue(auto2.reads().contains(base));
 
@@ -850,6 +838,7 @@ public class AutomaticModulesTest {
         assertTrue(auto3.reads().contains(m2));
         assertTrue(auto3.reads().contains(auto1));
         assertTrue(auto3.reads().contains(auto2));
+        assertFalse(auto3.reads().contains(auto3));
         assertTrue(auto3.reads().contains(base));
     }
 
@@ -859,6 +848,7 @@ public class AutomaticModulesTest {
      * automatic modules that are found by the after finder and not shadowed
      * by the before finder, or parent configurations, should also be resolved.
      */
+    @Test
     public void testInConfiguration6() throws IOException {
         // m1 requires auto1
         ModuleDescriptor descriptor1
@@ -918,12 +908,14 @@ public class AutomaticModulesTest {
 
         assertTrue(auto2.reads().contains(m1));
         assertTrue(auto2.reads().contains(auto1));
+        assertFalse(auto2.reads().contains(auto2));
         assertTrue(auto2.reads().contains(auto3));
         assertTrue(auto2.reads().contains(base));
 
         assertTrue(auto3.reads().contains(m1));
         assertTrue(auto3.reads().contains(auto1));
         assertTrue(auto3.reads().contains(auto2));
+        assertFalse(auto3.reads().contains(auto3));
         assertTrue(auto3.reads().contains(base));
     }
 
@@ -933,6 +925,7 @@ public class AutomaticModulesTest {
      * automatic module in a parent configuration then it should read all
      * automatic modules in the parent configuration.
      */
+    @Test
     public void testInConfiguration7() throws Exception {
         // m1 requires auto1
         ModuleDescriptor descriptor1 = ModuleDescriptor.newModule("m1")
@@ -964,11 +957,13 @@ public class AutomaticModulesTest {
 
         assertTrue(auto1.reads().contains(base));
         assertTrue(auto1.reads().contains(m1));
+        assertFalse(auto1.reads().contains(auto1));
         assertTrue(auto1.reads().contains(auto2));
 
         assertTrue(auto2.reads().contains(base));
         assertTrue(auto2.reads().contains(m1));
         assertTrue(auto2.reads().contains(auto1));
+        assertFalse(auto2.reads().contains(auto2));
 
         // m2 requires auto1
         ModuleDescriptor descriptor2 = ModuleDescriptor.newModule("m2")
@@ -999,7 +994,7 @@ public class AutomaticModulesTest {
      *   b* contains p
      *   c* contains p
      */
-    @Test(expectedExceptions = { ResolutionException.class })
+    @Test
     public void testDuplicateSuppliers1() throws IOException {
         ModuleDescriptor descriptor
             = ModuleDescriptor.newModule("a")
@@ -1018,7 +1013,7 @@ public class AutomaticModulesTest {
                                    ModuleFinder.of(dir));
 
         Configuration parent = ModuleLayer.boot().configuration();
-        resolve(parent, finder, "a");
+        assertThrows(ResolutionException.class, () -> resolve(parent, finder, "a"));
     }
 
     /**
@@ -1026,7 +1021,7 @@ public class AutomaticModulesTest {
      *   a contains p, requires b*
      *   b* contains p
      */
-    @Test(expectedExceptions = { ResolutionException.class })
+    @Test
     public void testDuplicateSuppliers2() throws IOException {
         ModuleDescriptor descriptor
             = ModuleDescriptor.newModule("a")
@@ -1044,12 +1039,13 @@ public class AutomaticModulesTest {
                                    ModuleFinder.of(dir));
 
         Configuration parent = ModuleLayer.boot().configuration();
-        resolve(parent, finder, "a");
+        assertThrows(ResolutionException.class, () -> resolve(parent, finder, "a"));
     }
 
     /**
      * Basic test of layer containing automatic modules
      */
+    @Test
     public void testInLayer() throws IOException {
         ModuleDescriptor descriptor
             = ModuleDescriptor.newModule("a")
@@ -1091,6 +1087,7 @@ public class AutomaticModulesTest {
     /**
      * Test miscellaneous methods.
      */
+    @Test
     public void testMisc() throws IOException {
         Path dir = Files.createTempDirectory(USER_DIR, "mods");
         Path m_jar = createDummyJarFile(dir.resolve("m.jar"), "p/T.class");
@@ -1132,13 +1129,10 @@ public class AutomaticModulesTest {
      * configuration.
      */
     static void testReadAllBootModules(Configuration cf, String mn) {
-
         Set<String> bootModules = ModuleLayer.boot().modules().stream()
                 .map(Module::getName)
                 .collect(Collectors.toSet());
-
         bootModules.forEach(other -> assertTrue(reads(cf, mn, other)));
-
     }
 
     /**
@@ -1202,5 +1196,4 @@ public class AutomaticModulesTest {
     {
         return createDummyJarFile(jarfile, null, entries);
     }
-
 }