JDK-8273154: Provide a JavadocTester method for non-overlapping, unordered output matching #5743

jonathan-gibbons · 2021-09-28T23:34:21Z

Please review a moderately simple improvement for JavadocTester and a related new test.

A new OutputChecker class is introduced that mostly supersedes the existing methods to check the output generated by javadoc and the standard doclet. A self-imposed restriction is that no existing tests are modified.

The new class can be used to check files generated by the doclet and the streams written by the tool. It can be configured to check for ordered output or not, overlapping output, and complete coverage, and can search for literal strings and regular expressions.

There is a corresponding new test which is a non-standard use of JavadocTester, since it is designed to test JavadocTester itself, and not javadoc or the doclet. (Quis custodiet ipsos custodes?) Various methods are overridden so that the operation of the underlying methods can be checked.

Although it is a goal to NOT modify the code of any existing tests, it turns out to be reasonable to adapt some of the existing check... methods to use the new OutputChecker. All javadoc tests pass, both locally and on all standard platforms. Many/most uses of the existing checkOutput method provide "ordered" strings, and are candidates to use the new ordered check. But enough uses are not ordered, so it is not reasonable to change the default at this time. It is noted as a TODO to examine the appropriate test cases, so that we can decide whether to fix those tests and change the default.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8273154: Provide a JavadocTester method for non-overlapping, unordered output matching

Reviewers

Pavel Rappo (@pavelrappo - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/5743/head:pull/5743
$ git checkout pull/5743

Update a local copy of the PR:
$ git checkout pull/5743
$ git pull https://git.openjdk.java.net/jdk pull/5743/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 5743

View PR using the GUI difftool:
$ git pr show -t 5743

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/5743.diff

…dered output matching

bridgekeeper · 2021-09-28T23:35:49Z

👋 Welcome back jjg! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2021-09-28T23:48:19Z

@jonathan-gibbons The following label will be automatically applied to this pull request:

javadoc

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

jonathan-gibbons · 2021-09-28T23:53:37Z

Reviewing the JBS issue, I see I did not address the following, which had been my intent to do so. I will update the PR.

// TODO: perhaps this method could be added to JavadocTester 
private void checkOutputEither(Output out, String first, String... other) { 
    checking("checkOutputEither"); 
    String output = getOutput(out);

mlbridge · 2021-09-28T23:55:47Z

Webrevs

pavelrappo · 2021-10-01T12:04:53Z

Unrelated to this PR. Every time I see these lines in JavadocTester, I cannot help thinking that we could use more specific APIs:

    public static final String FS = System.getProperty("file.separator");
    public static final String PS = System.getProperty("path.separator");
    public static final String NL = System.getProperty("line.separator");
    public static final String thisRelease = System.getProperty("java.specification.version");

The above lines can be translated into:

    public static final String FS = java.io.File.separator;
    public static final String PS = java.io.File.pathSeparator;
    public static final String NL = System.lineSeparator();
    public static final String thisRelease = String.valueOf(Runtime.version().feature());

I note that neither PS nor thisRelease seem to be currently used.

pavelrappo

Thanks a lot for doing this, Jon. Do you think you could also address closely related JDK-8270836 in this PR? Meanwhile, I'll keep reviewing the code.