On Tue, 24 May 2022 18:39:06 GMT, Pavel Rappo <pra...@openjdk.org> wrote:
>> Please review a test-only fix to make a recent new test more robust.
>>
>> The test was not fundamentally broken, so the essential functionality
>> remains the same. However, it does assume access to the `src/jdk.javadoc`
>> directory, and crashed when that was not available.
>>
>> The mitigation is two-fold:
>>
>> 1. introduce and use a new `SnippetUtils.ConfigurationException` that is
>> thrown when the test cannot find the necessary directories.
>> 2. introduce and use 2 new test-suite keywords, `needs-src`
>> `needs-src-jdk_javadoc` that can be used on the jtreg command-line to filter
>> out this test, and any similar tests in the future.
>>
>> Tested locally, manually, by temporarily renaming key directories, to test
>> the different code paths. In all cases, if the test is run and any
>> necessary directories are missing, the test _will still fail_, but now with
>> a more useful and specific exception and detail message.
>
> test/langtools/jdk/javadoc/doclet/testDocletExample/TestDocletExample.java
> line 71:
>
>> 69: var entryPointSnippet = snippets.getSnippetById(dc,
>> "entry-point");
>> 70: if (entryPointSnippet == null) {
>> 71: throw new Error("Cannot find snippet \"entry-point\"");
>
> I understand it as follows. Although this code now throws generic `Error`
> instead of `NullPointerException`, which the bug reporter observed, we
> shouldn't see that `Error` in circumstances similar to those of bug reporter.
> This is because in those circumstances the code will throw
> `ConfigurationException` earlier, at construction time, so we won't reach
> this check. Do I understand it correctly?
Yes. Although we could throw NPE even here, I was wanting to throw something
that indicates the test is dysfunctional, as compared to failing or crashing.
> test/langtools/jdk/javadoc/doclet/testDocletExample/TestDocletExample.java
> line 97:
>
>> 95: var dc = snippets.getDocTrees().getDocCommentTree(docletPkg);
>> 96: var exampleSnippet = snippets.getSnippetById(dc, "Example.java");
>> 97: if (exampleSnippet == null) {
>
> Same as above.
Yes, same reasoning. `SnippetUtils` is broken if we get a null result.
-------------
PR: https://git.openjdk.java.net/jdk/pull/8796
