Skip to content

Commit

Permalink
8290126: Add a check in JavadocTester for "javadoc should not crash"
Browse files Browse the repository at this point in the history
Reviewed-by: prappo
  • Loading branch information
jonathan-gibbons committed Sep 1, 2022
1 parent 0a4d0ce commit 0fb9469
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ public void test() {
testSrc("DocTest.java")
);
checkExit(Exit.OK);

// javadoc does not report an exit code for an internal exception (!)
// so monitor stderr for stack dumps.
checkOutput(Output.STDERR, false, "at com.sun");
}

/**
Expand Down
19 changes: 16 additions & 3 deletions test/langtools/jdk/javadoc/lib/javadoc/tester/JavadocTester.java
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ public enum DirectoryCheck {
NONE(null) { @Override void check(Path dir) { } };

/** The filter used to detect that files should <i>not</i> be present. */
DirectoryStream.Filter<Path> filter;
private final DirectoryStream.Filter<Path> filter;

DirectoryCheck(DirectoryStream.Filter<Path> f) {
filter = f;
Expand Down Expand Up @@ -246,6 +246,7 @@ void check(Path dir) {
private boolean automaticCheckAccessibility = true;
private boolean automaticCheckLinks = true;
private boolean automaticCheckUniqueOUT = true;
private boolean automaticCheckNoStacktrace = true;
private boolean useStandardStreams = false;
private StandardJavaFileManager fileManager = null;

Expand Down Expand Up @@ -488,6 +489,11 @@ public void javadoc(String... args) {
}
});

if (automaticCheckNoStacktrace) {
// Any stacktrace will have javadoc near the bottom of the stack
checkOutput(Output.STDERR, false, "at jdk.javadoc/jdk.javadoc.internal.");
}

if (exitCode == Exit.OK.code && Files.exists(outputDir)) {
if (automaticCheckLinks) {
checkLinks();
Expand Down Expand Up @@ -533,6 +539,13 @@ public void setAutomaticCheckUniqueOUT(boolean b) {
automaticCheckUniqueOUT = b;
}

/**
* Sets whether or not to check for stacktraces.
*/
public void setAutomaticCheckNoStacktrace(boolean b) {
automaticCheckNoStacktrace = b;
}

/**
* Sets whether to use standard output streams (stdout and stderr)
* instead of a single temporary stream.
Expand Down Expand Up @@ -1147,7 +1160,7 @@ public OutputChecker setAllowOverlaps(boolean allowOverlaps) {
public OutputChecker check(String... strings) {
if (name == null) {
out.println("Skipping checks for:" + NL
+ List.of(strings).stream()
+ Stream.of(strings)
.map(s -> " " + toShortString(s))
.collect(Collectors.joining(NL)));
return this;
Expand All @@ -1169,7 +1182,7 @@ public OutputChecker check(String... strings) {
public OutputChecker check(Pattern... patterns) {
if (name == null) {
out.println("Skipping checks for:" + NL
+ List.of(patterns).stream()
+ Stream.of(patterns)
.map(p -> " " + toShortString(p.pattern()))
.collect(Collectors.joining(NL)));
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public static void main(String... args) throws Exception {
tester.setup().runTests();
}

private final List<String> messages = new ArrayList<>();
protected final List<String> messages = new ArrayList<>();
private int testErrors = 0;

/**
Expand Down Expand Up @@ -158,7 +158,7 @@ private void report(String message) {

//-------------------------------------------------

private final ToolBox tb = new ToolBox();
protected final ToolBox tb = new ToolBox();

TestJavadocTester setup() throws IOException {
Path src = Path.of("src");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/*
* Copyright (c) 2022, 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 8290126
* @summary Add a check in JavadocTester for "javadoc should not crash"
* @library /tools/lib/ ../lib
* @modules jdk.javadoc/jdk.javadoc.internal.tool
* @build toolbox.ToolBox javadoc.tester.* TestJavadocTester
* @run main TestJavadocTesterCrash
*/

import com.sun.source.doctree.DocTree;
import jdk.javadoc.doclet.Taglet;

import javax.lang.model.element.Element;
import java.nio.file.Path;
import java.util.EnumSet;
import java.util.List;
import java.util.Set;

/**
* Tests that {@code JavadocTester} detects and reports exceptions.
*
* It is not a direct test of the javadoc tool or the output generated by the
* Standard Doclet, although both are indirectly used as part of this test.
*
* The test uses the infrastructure of TestJavadocTester, but cannot be
* added to the tests there, which rely on checking aspects of the output
* from a single run on javadoc. This test forces a crash to occur in
* javadoc, and verifies that JavadocTester detects and reports the crash.
*
* Arguably, a crash in a user-provided taglet should not cause a full stack
* trace. If that is ever fixed, we would need to revisit the goals and mechanism
* of this test.
*/
public class TestJavadocTesterCrash extends TestJavadocTester {
public static void main(String... args) throws Exception {
TestJavadocTesterCrash tester = new TestJavadocTesterCrash();
tester.runTests();
}

/** A taglet that can throw an exception. */
public static class TestTaglet implements Taglet {
public TestTaglet() { }

@Override
public Set<Location> getAllowedLocations() {
return EnumSet.allOf(Location.class);
}

@Override
public boolean isInlineTag() {
return true;
}

@Override
public String getName() {
return "test-taglet";
}

@Override
public String toString(List<? extends DocTree> tags, Element element) {
String s = tags.toString();
if (s.contains("test")) {
throw new Error("demo error");
};
return s;
}
}

@Test
public void testDetectException(Path base) throws Exception {
messages.clear();
Path src = base.resolve("src");
tb.writeJavaFiles(src,
"""
package p;
/** Comment.
abc {@test-taglet simple} {@test-taglet test} xyz
*/
public class C { }""");
javadoc("-d", base.resolve("api").toString(),
"-tagletpath", System.getProperty("test.class.path"),
"-taglet", TestTaglet.class.getName(),
"-sourcepath", src.toString(),
"p");
checkExit(Exit.ERROR);

// verify that the taglet threw an exception as intended
new OutputChecker(Output.OUT)
.setExpectOrdered(true)
.check("Generating testDetectException/api/p/C.html...",
"error: An internal exception has occurred.",
"(java.lang.Error: demo error)",
"1 error");

// verify that JavadocTester detected the crash
checkMessages("FAILED: STDERR: following text found:");
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, 2022, 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
Expand Down Expand Up @@ -161,6 +161,7 @@ public void testFileManagerAccess(Path base) throws Exception {

try {
setFileManager(tfm);
setAutomaticCheckNoStacktrace(false);
javadoc("-d", base.resolve("api").toString(),
"-sourcepath", srcDir.toString(),
"p");
Expand All @@ -173,6 +174,7 @@ public void testFileManagerAccess(Path base) throws Exception {
.replace("##EXC##", TestException.class.getName()));
} finally {
setFileManager(null);
setAutomaticCheckNoStacktrace(true);
}
}
}
Expand All @@ -190,6 +192,7 @@ public void testFileObjectRead(Path base) throws Exception {

try {
setFileManager(tfm);
setAutomaticCheckNoStacktrace(false);
javadoc("-d", base.resolve("api").toString(),
"-sourcepath", srcDir.toString(),
"p");
Expand All @@ -203,6 +206,7 @@ public void testFileObjectRead(Path base) throws Exception {
.replace("##FILE##", srcDir.resolve("p").resolve("C.java").toString()));
} finally {
setFileManager(null);
setAutomaticCheckNoStacktrace(true);
}
}
}
Expand All @@ -222,6 +226,7 @@ public void testFileObjectWrite(Path base) throws Exception {

try {
setFileManager(tfm);
setAutomaticCheckNoStacktrace(false);
javadoc("-d", outDir.toString(),
"-sourcepath", srcDir.toString(),
"p");
Expand All @@ -236,6 +241,7 @@ public void testFileObjectWrite(Path base) throws Exception {
.replace("##FILE##", outDir.resolve("p").resolve("C.html").toString()));
} finally {
setFileManager(null);
setAutomaticCheckNoStacktrace(true);
}
}
}
Expand Down

1 comment on commit 0fb9469

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.