RFR: 8273242: (test) Refactor to use TestNG for RuntimeTests ExecCommand tests
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. ------------- Commit messages: - 8273242: Refactored ExecCommand to use TestNG DataProvider for cases Changes: https://git.openjdk.java.net/jdk/pull/5335/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5335&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273242 Stats: 270 lines in 1 file changed: 137 ins; 97 del; 36 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
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.
LGTM. test/jdk/java/lang/RuntimeTests/exec/ExecCommand.java line 270:
268: } 269: } 270: }
No newline at the eof. ------------- Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5335
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
On Tue, 7 Sep 2021 22:01:54 GMT, Lance Andersen <lancea@openjdk.org> wrote:
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
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....
That code is pre-existing. Your suggestion can completely replace the deleteOut method. Tnx ------------- 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
On Wed, 8 Sep 2021 19:31:46 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.
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
Marked as reviewed by lancea (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/5335
On Wed, 8 Sep 2021 19:31:46 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.
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
Marked as reviewed by naoto (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/5335
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.
This pull request has now been integrated. Changeset: 7fd6b0bf Author: Roger Riggs <rriggs@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/7fd6b0bfd8ab3c64b374c71010bdfa369f0c... Stats: 288 lines in 1 file changed: 149 ins; 105 del; 34 mod 8273242: (test) Refactor to use TestNG for RuntimeTests ExecCommand tests Reviewed-by: naoto, lancea ------------- PR: https://git.openjdk.java.net/jdk/pull/5335
participants (3)
-
Lance Andersen
-
Naoto Sato
-
Roger Riggs