Generally very good.
As a suggestion for an optional improvement, the new test is OK, but
given the use of system properties, better test hygiene would be to set
a new properties object, and remove it when the test is complete. This
could be done in a try-finally block around the javadoc invocation.
Given this comment in the test:
146 /* TODO: Ideally, these system properties should be passed to the
`javadoc` method below.
147 However, as of today there's no such option, so for now this
is hardcoded */
I don't anticipate adding support for system properties
into|JavadocTester|since this test is a specific corner case where you
can't pass values to the doclet via options, which would be a better
methodology for any other puppet doclet being used to test non-option
features. What you have done is more than good enough to pass
out-of-band data, and the suggestion to temporarily install/remove a new
properties would be even better. Best of all would be to be able to
pass a configured instance of a doclet into|JavadocTester|and from there
into an instance of the javadoc tool via its API ... but that last step
is not yet available.
-- Jon
On 4/29/20 11:02 AM, Pavel Rappo wrote:
Hello,
Please review the change for https://bugs.openjdk.java.net/browse/JDK-8224613
http://cr.openjdk.java.net/~prappo/8224613/webrev.00/
This addresses a scenario in which a doclet failure was previously overlooked.
More specifically, if a doclet returned false after processing one or more of
its options while otherwise staying silent, the tool continued its execution.
-Pavel
