On Fri, 1 Dec 2023 11:09:30 GMT, Stefan Karlsson <stef...@openjdk.org> wrote:
>> Can I please get a review for this change to the test library's >> `OutputAnalyzer` class, which proposes to remove some unnecessary logging >> from the `getExitValue()` call? >> >> As noted in https://bugs.openjdk.org/browse/JDK-8321163, right now this >> method logs: >> >> >> [2023-11-24T11:47:54.557561Z] Waiting for completion for process 24909 >> [2023-11-24T11:47:54.557873Z] Waiting for completion finished for process >> 24909 >> >> >> even when the process has already completed and the exit value already >> known. The change in this PR makes it such that if the exit value is >> available then we no longer log this (nor call `process.waitFor()`). >> >> No new tests have been added given the nature of this change. tier1, tier2 >> and tier3 tests continue to pass with this change. > > test/lib/jdk/test/lib/process/OutputBuffer.java line 158: > >> 156: boolean aborted = true; >> 157: try { >> 158: this.processExitCode = exitCode = p.waitFor(); > > According to the `waitFor` javadocs it returns the "exit value" and this > function is named `getExitValue()`. I propose that we rename > `processExitCode` to `exitValue` (alt. `processExitValue`). With the earlier suggestion, this would become: exitValue = p.waitFor(); ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16919#discussion_r1411949002
