On Wed, 1 Sep 2021 16:37:24 GMT, Roger Riggs <rriggs@openjdk.org> wrote:

The ExecCommand test of Runtime.exec is difficult to maintain; the parallel arrays are hard to keep in sync. This cleanup converts to use TestNG DataProviders and other improvements.

Looks good Roger. A couple trivial questions/suggestions below but feel free to ignore :-) test/jdk/java/lang/RuntimeTests/exec/ExecCommand.java line 107:

105: private static void deleteOut(String path) { 106: try { 107: Files.delete(FileSystems.getDefault().getPath(path));

More of a curious question, is there a reason you couldn't use Files::deleteIfExists or Path.of() instead of FileSystems.getDefault.... test/jdk/java/lang/RuntimeTests/exec/ExecCommand.java line 199:

197: System.setSecurityManager(null); 198: 199: testCommandMode(command, "Ambiguous Unset", testFile, perModeExpected.get(0));

Any thought to creating a constant for the index value for perModeExpected.get(XX) for extra clarity? ------------- Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5335

The ExecCommand test of Runtime.exec is difficult to maintain; the parallel arrays are hard to keep in sync. This cleanup converts to use TestNG DataProviders and other improvements.

Roger Riggs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Simplify file deletion Add enum to document test modes. - Merge branch 'master' into 8273242-execcommand-refactor - 8273242: Refactored ExecCommand to use TestNG DataProvider for cases ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/5335/files - new: https://git.openjdk.java.net/jdk/pull/5335/files/d374449b..8dbd021e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5335&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5335&range=00-01 Stats: 35582 lines in 1179 files changed: 24923 ins; 5611 del; 5048 mod Patch: https://git.openjdk.java.net/jdk/pull/5335.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5335/head:pull/5335 PR: https://git.openjdk.java.net/jdk/pull/5335