On Sat, 7 Mar 2026 13:49:47 GMT, Roger Riggs <[email protected]> wrote:

> Refactor ProcessBuilder and ProcessHandle tests to use Junit instead of 
> TestNG.
> Remove old main testng driver stubs that allowed tests to be run from the 
> command line.

test/jdk/java/lang/ProcessHandle/OnExitTest.java line 207:

> 205:             Assertions.assertFalse(proc.isAlive(), "destroyed process is 
> alive:: %s%n" + proc);
> 206:         } catch (IOException | InterruptedException ex) {
> 207:             Assertions.fail(ex.getMessage());

Hello Roger, this is pre-existing, but it might be good to include the original 
exception too, like it's done in other places in this test


Assertions.fail(ex.getMessage(), ex);

test/jdk/java/lang/ProcessHandle/TreeTest.java line 443:

> 441:                     .collect(Collectors.toList()));
> 442: 
> 443:             Assertions.assertEquals(                    factor, 
> getChildren(p1Handle).size(), "expected direct children");

Nit - looks like unnecessary whitespace between `(` and `factor`.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/30128#discussion_r2902799379
PR Review Comment: https://git.openjdk.org/jdk/pull/30128#discussion_r2902792608

Reply via email to