On Tue, 16 Nov 2021 12:27:16 GMT, Pavel Rappo <[email protected]> wrote:
>> Please review a moderately simple addition to the `JavadocTester` world, to
>> be able to modify the behavior of a file manager to throw exceptions as
>> needed.
>>
>> The bulk of the new functionality is in a new class,
>> `TestJavaFileManagerBuilder`, which uses the builder pattern to allow a
>> client to configure and build a file manager that can provide file objects
>> for which the behavior can be overridden for one or more methods. The
>> expected use case is to throw an exception instead of calling the underlying
>> delegate method.
>>
>> `JavadocTester` is modified to allow a file manager to be provided when
>> invoking `javadoc`. This requires some minor changes to the outermost
>> javadoc tool classes, `Main` and `Start`. Rather than add more `static`
>> methods to `Main`, instance methods are now provided to make it easier to
>> configure the values that will be passed to `Start`. In `Start`, it was
>> previously assumed that either the default file manager was being used or
>> that _all_ paths would be configured directly in the file manager. The
>> latter part of that assumption is reduced so that path-setting options (e.g.
>> `--source-path`, `--class-path` etc) can be passed to the provided file
>> manager. (Previously, they were silently ignored.) It is an error to pass
>> path-like options as javadoc options to a file manager that does not support
>> them. However, since none of the changes are visible to any public API,
>> this should not be an issue.
>>
>> A new test is provided, with a few simple test cases. One is for direct use
>> of the new file manager mechanism (without using `javadoc`). The other two
>> illustrate how the feature can be used inside a `JavadocTester` call of
>> `javadoc`. Given that the feature is a relatively simple combination of
>> predicates and proxies, it's not clear that we need a significant body of
>> test cases.
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/doclets.properties
> line 48:
>
>> 46: doclet.exception.read.resource=Error reading system resource: {0}\n\
>> 47: \t({1})
>> 48: doclet.internal.exception=An internal exception has occurred.\n\
>
> Out of curiosity, how did you notice that trailing space?
The message shows up in the new test. Both the trailing space and the tabs
caused problems in the string matching. I fixed the space here, as a one-off.
The tabs in this and related messages seem questionable and should maybe also
be addressed, but that seemed too much out of scope for this work.
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Main.java line 134:
>
>> 132: * Runs javadoc with preconfigured values and a given set of
>> arguments.
>> 133: * Any errors will be reported to the error stream, or to {@link
>> System#err}
>> 134: * if not error stream has been specified with {@code setStreams}.
>
> Suggestion:
>
> * if no error stream has been specified with {@code setStreams}.
Noted.
> test/langtools/jdk/javadoc/lib/javadoc/tester/JavadocTester.java line 381:
>
>> 379: : jdk.javadoc.internal.tool.Main.execute(args,
>> outOut.pw); // default
>> 380: */
>> 381: jdk.javadoc.internal.tool.Main main = new
>> jdk.javadoc.internal.tool.Main();
>
> Do we need this commented-out code?
Thanks; no; I'll remove it.
> test/langtools/jdk/javadoc/lib/javadoc/tester/TestJavaFileManagerBuilder.java
> line 66:
>
>> 64: * by method and predicate and then dynamically build the set of methods
>> to be used for
>> 65: * a file object by filtering the methods by their applicable predicate.
>> 66: *
>
> Suggestion:
Noted.
-------------
PR: https://git.openjdk.java.net/jdk/pull/6404
