From 340a3b4ffbf53b6ab0b5ee1cc834582b85107039 Mon Sep 17 00:00:00 2001 From: Hannes Wallnoefer Date: Thu, 18 Apr 2024 13:34:59 +0200 Subject: [PATCH 1/3] JDK-8294880: Review running time of jdk/internal/shellsupport/doc/JavadocHelperTest.java --- test/langtools/TEST.groups | 5 +- .../doc/FullJavadocHelperTest.java | 119 ++++++++++++++++++ .../shellsupport/doc/JavadocHelperTest.java | 45 +++---- 3 files changed, 138 insertions(+), 31 deletions(-) create mode 100644 test/langtools/jdk/internal/shellsupport/doc/FullJavadocHelperTest.java diff --git a/test/langtools/TEST.groups b/test/langtools/TEST.groups index e1ffd5d530be1..8009ef6ffe70a 100644 --- a/test/langtools/TEST.groups +++ b/test/langtools/TEST.groups @@ -1,4 +1,4 @@ -# Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2015, 2024, 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 @@ -53,7 +53,8 @@ langtools_jshell_unstable = \ jdk/jshell/ToolLocaleMessageTest.java \ jdk/jshell/ToolReloadTest.java \ jdk/jshell/UserInputTest.java \ - jdk/jshell/UserJdiUserRemoteTest.java + jdk/jshell/UserJdiUserRemoteTest.java \ + jdk/internal/shellsupport/doc/FullJavadocHelperTest.java langtools_jdeprscan = \ tools/all \ diff --git a/test/langtools/jdk/internal/shellsupport/doc/FullJavadocHelperTest.java b/test/langtools/jdk/internal/shellsupport/doc/FullJavadocHelperTest.java new file mode 100644 index 0000000000000..e05d132ba7bbd --- /dev/null +++ b/test/langtools/jdk/internal/shellsupport/doc/FullJavadocHelperTest.java @@ -0,0 +1,119 @@ +/* + * Copyright (c) 2017, 2024, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8189778 + * @summary Test JavadocHelper + * @library /tools/lib + * @modules jdk.compiler/com.sun.tools.javac.api + * jdk.compiler/com.sun.tools.javac.main + * jdk.compiler/jdk.internal.shellsupport.doc + * @build toolbox.ToolBox toolbox.JarTask toolbox.JavacTask + * @run testng/timeout=900/othervm -Xmx1024m FullJavadocHelperTest + */ + +import java.io.IOException; +import java.net.URI; +import java.nio.file.DirectoryStream; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + + +import javax.lang.model.element.Element; +import javax.lang.model.element.ModuleElement; +import javax.lang.model.element.ModuleElement.ExportsDirective; +import javax.lang.model.element.TypeElement; +import javax.lang.model.util.ElementFilter; +import javax.tools.Diagnostic.Kind; +import javax.tools.DiagnosticListener; +import javax.tools.JavaCompiler; +import javax.tools.JavaFileObject; +import javax.tools.StandardJavaFileManager; +import javax.tools.ToolProvider; + +import com.sun.source.util.JavacTask; +import jdk.internal.shellsupport.doc.JavadocHelper; +import org.testng.annotations.Test; + +@Test +public class FullJavadocHelperTest { + + /* + * Long-running test to retrieve doc comments for enclosed elements of all JDK classes. + */ + public void testAllDocs() throws IOException { + JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + DiagnosticListener noErrors = d -> { + if (d.getKind() == Kind.ERROR) { + throw new AssertionError(d.getMessage(null)); + } + }; + + List sources = new ArrayList<>(); + Path home = Paths.get(System.getProperty("java.home")); + Path srcZip = home.resolve("lib").resolve("src.zip"); + if (Files.isReadable(srcZip)) { + URI uri = URI.create("jar:" + srcZip.toUri()); + try (FileSystem zipFO = FileSystems.newFileSystem(uri, Collections.emptyMap())) { + Path root = zipFO.getRootDirectories().iterator().next(); + + //modular format: + try (DirectoryStream ds = Files.newDirectoryStream(root)) { + for (Path p : ds) { + if (Files.isDirectory(p)) { + sources.add(p); + } + } + } + try (StandardJavaFileManager fm = + compiler.getStandardFileManager(null, null, null)) { + JavacTask task = + (JavacTask) compiler.getTask(null, fm, noErrors, null, null, null); + task.getElements().getTypeElement("java.lang.Object"); + for (ModuleElement me : task.getElements().getAllModuleElements()) { + List exports = + ElementFilter.exportsIn(me.getDirectives()); + for (ExportsDirective ed : exports) { + try (JavadocHelper helper = JavadocHelper.create(task, sources)) { + List content = + ed.getPackage().getEnclosedElements(); + for (TypeElement clazz : ElementFilter.typesIn(content)) { + for (Element el : clazz.getEnclosedElements()) { + helper.getResolvedDocComment(el); + } + } + } + } + } + } + } + } + } +} diff --git a/test/langtools/jdk/internal/shellsupport/doc/JavadocHelperTest.java b/test/langtools/jdk/internal/shellsupport/doc/JavadocHelperTest.java index d8539b8c656c7..cf1b966f6f434 100644 --- a/test/langtools/jdk/internal/shellsupport/doc/JavadocHelperTest.java +++ b/test/langtools/jdk/internal/shellsupport/doc/JavadocHelperTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2024, 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 @@ -30,19 +30,17 @@ * jdk.compiler/com.sun.tools.javac.main * jdk.compiler/jdk.internal.shellsupport.doc * @build toolbox.ToolBox toolbox.JarTask toolbox.JavacTask - * @run testng/timeout=900/othervm -Xmx1024m JavadocHelperTest + * @run testng JavadocHelperTest */ import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; -import java.nio.file.DirectoryStream; import java.nio.file.FileSystem; import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -51,8 +49,7 @@ import java.util.jar.JarOutputStream; import javax.lang.model.element.Element; -import javax.lang.model.element.ModuleElement; -import javax.lang.model.element.ModuleElement.ExportsDirective; +import javax.lang.model.element.Modifier; import javax.lang.model.element.TypeElement; import javax.lang.model.util.ElementFilter; import javax.tools.Diagnostic.Kind; @@ -68,6 +65,7 @@ import jdk.internal.shellsupport.doc.JavadocHelper; import org.testng.annotations.Test; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertTrue; @Test @@ -383,7 +381,11 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept } - public void testAllDocs() throws IOException { + /* + * This retrieves the doc comments of java.lang.StringBuilder members, which is where + * the crash in JDK-8189778 occurred. Full test moved to FullJavaDocHelperTest.java. + */ + public void testStringBuilderDocs() throws IOException { JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); DiagnosticListener noErrors = d -> { if (d.getKind() == Kind.ERROR) { @@ -391,39 +393,24 @@ public void testAllDocs() throws IOException { } }; - List sources = new ArrayList<>(); Path home = Paths.get(System.getProperty("java.home")); Path srcZip = home.resolve("lib").resolve("src.zip"); if (Files.isReadable(srcZip)) { URI uri = URI.create("jar:" + srcZip.toUri()); try (FileSystem zipFO = FileSystems.newFileSystem(uri, Collections.emptyMap())) { Path root = zipFO.getRootDirectories().iterator().next(); + List sources = List.of(root.resolve("java.base")); - //modular format: - try (DirectoryStream ds = Files.newDirectoryStream(root)) { - for (Path p : ds) { - if (Files.isDirectory(p)) { - sources.add(p); - } - } - } try (StandardJavaFileManager fm = compiler.getStandardFileManager(null, null, null)) { JavacTask task = (JavacTask) compiler.getTask(null, fm, noErrors, null, null, null); - task.getElements().getTypeElement("java.lang.Object"); - for (ModuleElement me : task.getElements().getAllModuleElements()) { - List exports = - ElementFilter.exportsIn(me.getDirectives()); - for (ExportsDirective ed : exports) { - try (JavadocHelper helper = JavadocHelper.create(task, sources)) { - List content = - ed.getPackage().getEnclosedElements(); - for (TypeElement clazz : ElementFilter.typesIn(content)) { - for (Element el : clazz.getEnclosedElements()) { - helper.getResolvedDocComment(el); - } - } + TypeElement clazz = task.getElements().getTypeElement("java.lang.StringBuilder"); + try (JavadocHelper helper = JavadocHelper.create(task, sources)) { + for (Element el : clazz.getEnclosedElements()) { + // All StringBuilder members are currently documented, but exclude private to be safe + if (!el.getModifiers().contains(Modifier.PRIVATE)) { + assertNotNull(helper.getResolvedDocComment(el)); } } } From 7a45c94dda4815a351cd7f3f9c3571df370dfb6e Mon Sep 17 00:00:00 2001 From: Hannes Wallnoefer Date: Tue, 23 Apr 2024 18:21:02 +0200 Subject: [PATCH 2/3] Move FullJavaDocHelperTest.java into its own group --- test/langtools/TEST.groups | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/langtools/TEST.groups b/test/langtools/TEST.groups index 8009ef6ffe70a..74503501da9b9 100644 --- a/test/langtools/TEST.groups +++ b/test/langtools/TEST.groups @@ -53,8 +53,7 @@ langtools_jshell_unstable = \ jdk/jshell/ToolLocaleMessageTest.java \ jdk/jshell/ToolReloadTest.java \ jdk/jshell/UserInputTest.java \ - jdk/jshell/UserJdiUserRemoteTest.java \ - jdk/internal/shellsupport/doc/FullJavadocHelperTest.java + jdk/jshell/UserJdiUserRemoteTest.java langtools_jdeprscan = \ tools/all \ @@ -64,6 +63,9 @@ langtools_jdeps = \ tools/all \ tools/jdeps +langtools_slow = \ + jdk/internal/shellsupport/doc/FullJavadocHelperTest.java + # All tests all = \ @@ -79,11 +81,13 @@ tier1 = \ :langtools_all \ lib \ tools \ - -:langtools_jshell_unstable + -:langtools_jshell_unstable \ + -:langtools_slow # (Almost) no langtools tests are tier 2. tier2 = \ - :langtools_jshell_unstable + :langtools_jshell_unstable \ + :langtools_slow # No langtools tests are tier 3 either. tier3 = From d400cf1b66e6952ce65bb0c50b000e4fa3c40669 Mon Sep 17 00:00:00 2001 From: Hannes Wallnoefer Date: Fri, 10 May 2024 15:38:40 +0200 Subject: [PATCH 3/3] Change quick test to run on a random subset of classes --- .../doc/FullJavadocHelperTest.java | 72 +------------------ .../shellsupport/doc/JavadocHelperTest.java | 72 +++++++++++++++---- 2 files changed, 60 insertions(+), 84 deletions(-) diff --git a/test/langtools/jdk/internal/shellsupport/doc/FullJavadocHelperTest.java b/test/langtools/jdk/internal/shellsupport/doc/FullJavadocHelperTest.java index e05d132ba7bbd..729ac92143f61 100644 --- a/test/langtools/jdk/internal/shellsupport/doc/FullJavadocHelperTest.java +++ b/test/langtools/jdk/internal/shellsupport/doc/FullJavadocHelperTest.java @@ -34,32 +34,7 @@ */ import java.io.IOException; -import java.net.URI; -import java.nio.file.DirectoryStream; -import java.nio.file.FileSystem; -import java.nio.file.FileSystems; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - -import javax.lang.model.element.Element; -import javax.lang.model.element.ModuleElement; -import javax.lang.model.element.ModuleElement.ExportsDirective; -import javax.lang.model.element.TypeElement; -import javax.lang.model.util.ElementFilter; -import javax.tools.Diagnostic.Kind; -import javax.tools.DiagnosticListener; -import javax.tools.JavaCompiler; -import javax.tools.JavaFileObject; -import javax.tools.StandardJavaFileManager; -import javax.tools.ToolProvider; - -import com.sun.source.util.JavacTask; -import jdk.internal.shellsupport.doc.JavadocHelper; import org.testng.annotations.Test; @Test @@ -69,51 +44,6 @@ public class FullJavadocHelperTest { * Long-running test to retrieve doc comments for enclosed elements of all JDK classes. */ public void testAllDocs() throws IOException { - JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); - DiagnosticListener noErrors = d -> { - if (d.getKind() == Kind.ERROR) { - throw new AssertionError(d.getMessage(null)); - } - }; - - List sources = new ArrayList<>(); - Path home = Paths.get(System.getProperty("java.home")); - Path srcZip = home.resolve("lib").resolve("src.zip"); - if (Files.isReadable(srcZip)) { - URI uri = URI.create("jar:" + srcZip.toUri()); - try (FileSystem zipFO = FileSystems.newFileSystem(uri, Collections.emptyMap())) { - Path root = zipFO.getRootDirectories().iterator().next(); - - //modular format: - try (DirectoryStream ds = Files.newDirectoryStream(root)) { - for (Path p : ds) { - if (Files.isDirectory(p)) { - sources.add(p); - } - } - } - try (StandardJavaFileManager fm = - compiler.getStandardFileManager(null, null, null)) { - JavacTask task = - (JavacTask) compiler.getTask(null, fm, noErrors, null, null, null); - task.getElements().getTypeElement("java.lang.Object"); - for (ModuleElement me : task.getElements().getAllModuleElements()) { - List exports = - ElementFilter.exportsIn(me.getDirectives()); - for (ExportsDirective ed : exports) { - try (JavadocHelper helper = JavadocHelper.create(task, sources)) { - List content = - ed.getPackage().getEnclosedElements(); - for (TypeElement clazz : ElementFilter.typesIn(content)) { - for (Element el : clazz.getEnclosedElements()) { - helper.getResolvedDocComment(el); - } - } - } - } - } - } - } - } + new JavadocHelperTest().retrieveDocComments(Boolean.TRUE::booleanValue); } } diff --git a/test/langtools/jdk/internal/shellsupport/doc/JavadocHelperTest.java b/test/langtools/jdk/internal/shellsupport/doc/JavadocHelperTest.java index cf1b966f6f434..55d258788fed4 100644 --- a/test/langtools/jdk/internal/shellsupport/doc/JavadocHelperTest.java +++ b/test/langtools/jdk/internal/shellsupport/doc/JavadocHelperTest.java @@ -31,25 +31,31 @@ * jdk.compiler/jdk.internal.shellsupport.doc * @build toolbox.ToolBox toolbox.JarTask toolbox.JavacTask * @run testng JavadocHelperTest + * @key randomness */ import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.nio.file.DirectoryStream; import java.nio.file.FileSystem; import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Random; +import java.util.function.BooleanSupplier; import java.util.function.Function; import java.util.jar.JarEntry; import java.util.jar.JarOutputStream; import javax.lang.model.element.Element; -import javax.lang.model.element.Modifier; +import javax.lang.model.element.ModuleElement; +import javax.lang.model.element.ModuleElement.ExportsDirective; import javax.lang.model.element.TypeElement; import javax.lang.model.util.ElementFilter; import javax.tools.Diagnostic.Kind; @@ -65,7 +71,6 @@ import jdk.internal.shellsupport.doc.JavadocHelper; import org.testng.annotations.Test; import static org.testng.Assert.assertEquals; -import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertTrue; @Test @@ -381,11 +386,35 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept } + private static long getSeed() { + long seed; + try { + // Throws NumberFormatException if the property is undefined + seed = Long.parseLong(System.getProperty("seed")); + } catch (NumberFormatException e) { + seed = new Random().nextLong(); + } + System.out.println("Random Seed: " + seed); + return seed; + } + /* - * This retrieves the doc comments of java.lang.StringBuilder members, which is where - * the crash in JDK-8189778 occurred. Full test moved to FullJavaDocHelperTest.java. + * Retrieves doc comments for a random subset of JDK classes. + * Set the system property `seed` to a random seed to reproduce + * a specific run of this test. */ - public void testStringBuilderDocs() throws IOException { + public void testRandomDocs() throws IOException { + Random random = new Random(getSeed()); + // Run test on 2% of classes, which corresponds to ~ 140 classes + retrieveDocComments(() -> random.nextInt(100) < 2); + } + + /** + * Retrieve documentation of enclosed elements for some or all JDK classes. + * + * @param shouldTest oracle function to decide whether a class should be tested + */ + protected void retrieveDocComments(BooleanSupplier shouldTest) throws IOException { JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); DiagnosticListener noErrors = d -> { if (d.getKind() == Kind.ERROR) { @@ -393,24 +422,41 @@ public void testStringBuilderDocs() throws IOException { } }; + List sources = new ArrayList<>(); Path home = Paths.get(System.getProperty("java.home")); Path srcZip = home.resolve("lib").resolve("src.zip"); if (Files.isReadable(srcZip)) { URI uri = URI.create("jar:" + srcZip.toUri()); try (FileSystem zipFO = FileSystems.newFileSystem(uri, Collections.emptyMap())) { Path root = zipFO.getRootDirectories().iterator().next(); - List sources = List.of(root.resolve("java.base")); + //modular format: + try (DirectoryStream ds = Files.newDirectoryStream(root)) { + for (Path p : ds) { + if (Files.isDirectory(p)) { + sources.add(p); + } + } + } try (StandardJavaFileManager fm = - compiler.getStandardFileManager(null, null, null)) { + compiler.getStandardFileManager(null, null, null)) { JavacTask task = (JavacTask) compiler.getTask(null, fm, noErrors, null, null, null); - TypeElement clazz = task.getElements().getTypeElement("java.lang.StringBuilder"); - try (JavadocHelper helper = JavadocHelper.create(task, sources)) { - for (Element el : clazz.getEnclosedElements()) { - // All StringBuilder members are currently documented, but exclude private to be safe - if (!el.getModifiers().contains(Modifier.PRIVATE)) { - assertNotNull(helper.getResolvedDocComment(el)); + task.getElements().getTypeElement("java.lang.Object"); + for (ModuleElement me : task.getElements().getAllModuleElements()) { + List exports = + ElementFilter.exportsIn(me.getDirectives()); + for (ExportsDirective ed : exports) { + try (JavadocHelper helper = JavadocHelper.create(task, sources)) { + List content = + ed.getPackage().getEnclosedElements(); + for (TypeElement clazz : ElementFilter.typesIn(content)) { + if (shouldTest.getAsBoolean()) { + for (Element el : clazz.getEnclosedElements()) { + helper.getResolvedDocComment(el); + } + } + } } } }