RFR: 8198317: Enhance JavacTool.getTask for flexibility [v3]

Jonathan Gibbons jjg at openjdk.java.net
Tue Dec 29 03:45:57 UTC 2020

On Sat, 26 Dec 2020 18:58:15 GMT, Guoxiong Li <github.com+13688759+lgxbslgx at openjdk.org> wrote:

>> Hi all,
>> This little patch enhances the setting of `Log` in `JavacTool.getTask`.
>> Thank you for taking the time to review.
>> Best Regards.
> Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:
>   Revise test.

Any time that tests split a string into lines it is really important to ensure that it works on Windows as well as Linux/Mac platforms, because of the different line endings.

The code looks OK. Alternatively, if you are not concerned to check for the exact line ending sequence, you can use `split("\\R")` to split on any line ending sequence.  Either way, it's worth checking the test runs on platforms with both kinds of typical line ending.

test/langtools/tools/javac/api/8198317/T8198317.java line 90:

> 88:                 .call();
> 89:         tb.checkEqual(expected, Arrays.asList(bais.toString().split(lineSeparator)));
> 90:         System.setErr(prev);

Although not strictly necessary in this case, it is good practice to use `try ... finally` to reset values back to some previous value. In other words, the `System.setErr(prev);` should be in the `finally` clause.

test/langtools/tools/javac/api/8198317/T8198317.java line 82:

> 80: 
> 81:         // Situation: out is null and the value is not set in the context.
> 82:         ByteArrayOutputStream bais = new ByteArrayOutputStream();

The variable name `bais` is "surprising". Is it a cut-n-paste from elsewhere?    I was expecting to see `baos` as an acronym for "byte array output stream",  `bais` suggests "byte array input stream".


PR: https://git.openjdk.java.net/jdk/pull/1896

More information about the compiler-dev mailing list