On Fri, 5 Jan 2024 13:52:23 GMT, Pavel Rappo <pra...@openjdk.org> wrote:
> This PR improves diagnostic output and behaviour of the
> `TestMethodCommentsAlgorithm` test in environments that do not meet that
> test's expectations.
>
> The test assumes that a correct source file for `java.lang.Object` is
> available nearby. While the test verified that assumption, it didn't verify
> it deep enough. The issue described in the linked bug report seems to be that
> a directory that looks like a source directory does not contain that source
> file.
>
> The solution is to check the assumptions more thoroughly. Note that if the
> assumptions aren't met, the test will be skipped, but it will not fail. Some
> tools display a skipped test as **passed**, which could be misleading. If I
> were the original bug reporter, I'd investigate why the source file for
> `java.lang.Object` is missing.
>
> As for exception messages, I tried my best to make them helpful. That said,
> test exception messages are not user-level error messages. The stacktrace of
> an exception is supposed to be analysed in conjunction with the source that
> threw that exception.
test/langtools/jdk/javadoc/doclet/testMethodCommentAlgorithm/TestMethodCommentsAlgorithm.java
line 402:
> 400: start = Path.of(testSrc).toAbsolutePath();
> 401: } catch (InvalidPathException | IOError e) {
> 402: throw new SkippedException("Couldn't make sense of '" +
> testSrc + "'", e);
As a general stylistic convention, if you don't know much about the exception
being caught or how the "rethrown" exception will be handled, I like to include
the nested-exception message inr the rethrown message, as well as giving it in
the cause.
So, for example,
throw new SkippedException("Couldn't make sense of '" + testSrc + "': " + e, e);
(note the `: " + e`)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17280#discussion_r1443395133
