RFR: 8198317: Enhance JavacTool.getTask for flexibility [v3]
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:
> 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".
More information about the compiler-dev