On Wed, 20 Sep 2023 04:29:00 GMT, Daniel Jeliński <djelin...@openjdk.org> wrote:
>> Brian Burkhalter has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8315960: Fix indentation
>
> 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.
> 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.
> 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.
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15757#discussion_r1331832086
PR Review Comment: https://git.openjdk.org/jdk/pull/15757#discussion_r1331832598
PR Review Comment: https://git.openjdk.org/jdk/pull/15757#discussion_r1331834543
PR Review Comment: https://git.openjdk.org/jdk/pull/15757#discussion_r1331833630
