On Wed, 20 Sep 2023 15:42:27 GMT, Brian Burkhalter <b...@openjdk.org> wrote:
>> test/jdk/java/io/File/TempDirDoesNotExist.java line 153:
>>
>>> 151: @ParameterizedTest
>>> 152: @MethodSource("tempDirSource")
>>> 153: public void existingMessage(int exitValue, String errorMsg,
>>
>> `exitValue` is always zero, and `errorMsg` is always `WARNING`; do you need
>> to use parameters here?
>
> Right. I intentionally left it that way to match the original but it should
> probably be changed.
Extraneous parameters removed in b3ac8c7b1242281bda86f7dc98efccf2bcefc7f6.
>> test/jdk/java/io/File/TempDirDoesNotExist.java line 161:
>>
>>> 159: @ParameterizedTest
>>> 160: @MethodSource("noTempDirSource")
>>> 161: public void nonexistentMessage(int exitValue, String errorMsg,
>>
>> exitValue is always zero, and errorMsg is always WARNING; do you need to use
>> parameters here?
>
> Same comment as above re: line 100.
Extraneous parameters removed in b3ac8c7b1242281bda86f7dc98efccf2bcefc7f6.
>> test/jdk/java/io/File/TempDirDoesNotExist.java line 170:
>>
>>> 168: @MethodSource("counterSource")
>>> 169: public void messageCounter(int exitValue, String... options)
>>> 170: throws Exception {
>>
>> Suggestion:
>>
>> public void messageCounter(int exitValue, String... options)
>> throws Exception {
>
> Thanks; will fix.
Fixed in b3ac8c7b1242281bda86f7dc98efccf2bcefc7f6.
>> test/jdk/java/io/File/TempDirDoesNotExist.java line 174:
>>
>>> 172: List<String> list =
>>> originalOutput.asLines().stream().filter(line
>>> 173: -> line.equalsIgnoreCase(WARNING)).toList();
>>> 174: if (list.size() != 1 || originalOutput.getExitValue() !=
>>> exitValue)
>>
>> (preexisting) the exception message doesn't make much sense in the second
>> case (`originalOutput.getExitValue() != exitValue`)
>
> Will reconsider this.
Improved in b3ac8c7b1242281bda86f7dc98efccf2bcefc7f6.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15757#discussion_r1332215332
PR Review Comment: https://git.openjdk.org/jdk/pull/15757#discussion_r1332215487
PR Review Comment: https://git.openjdk.org/jdk/pull/15757#discussion_r1332215855
PR Review Comment: https://git.openjdk.org/jdk/pull/15757#discussion_r1332215701
