RFR: 8337111: Bad HTML checker for generated documentation [v2]

[Jonathan Gibbons]

On Mon, 26 Aug 2024 09:22:59 GMT, Nizar Benalla <nbenalla at openjdk.org> wrote:

>> Can I please get a review for this PR that adds 4 new html "Checkers" for the generated documentation.
>> More details are in the JBS issues
>> 
>> These tests were mostly inspired /converted from the existing [Doccheck](https://github.com/openjdk/doccheck).
>
> Nizar Benalla has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix path passed to the test

Good start!  Some suggestions inline

test/langtools/jdk/javadoc/doccheck/DocCheck.java line 43:

> 41: import java.util.concurrent.ExecutorService;
> 42: import java.util.concurrent.Executors;
> 43: import java.util.concurrent.Future;

It is often common practice to put Java SE and JDK imports first.

test/langtools/jdk/javadoc/doccheck/DocCheck.java line 56:

> 54:     @Before
> 55:     public void setUp() {
> 56:         Path root = Path.of(ROOT_PATH.getParent() + File.separator + "docs" + File.separator + System.getProperty("doccheck.dir"));

somewhat preferable to use `Path.resolve` instead of adding strings like this

test/langtools/jdk/javadoc/doccheck/DocCheck.java line 71:

> 69:                 BadCharacterChecker badChars = new BadCharacterChecker();
> 70:                 HtmlFileChecker docChecker = new HtmlFileChecker(new DocTypeChecker());
> 71:                 HtmlFileChecker htmlChecker = new HtmlFileChecker(new LinkChecker());

I suggest extracting these declarations into a method called something like `getCheckers()`
and then having two alternative methods to call them, called homelike like `runSerially(List<Checker> checkers)` and `runWithExecutor(List<Checker> checkers)`

-------------

PR Review: https://git.openjdk.org/jdk/pull/20711#pullrequestreview-2261580004
PR Review Comment: https://git.openjdk.org/jdk/pull/20711#discussion_r1731783076
PR Review Comment: https://git.openjdk.org/jdk/pull/20711#discussion_r1731784158
PR Review Comment: https://git.openjdk.org/jdk/pull/20711#discussion_r1731787106