RFR: JDK-8293877: Rewrite MineField test
Jonathan Gibbons
jjg at openjdk.org
Wed Sep 21 01:06:31 UTC 2022
Please review an update to convert the remaining langtools shell tests, in `test/langtools/tools/javac/Paths/*` from shell to Java code.
This will be an awkward/difficult/tedious (choose your adjective) review. The old files have been deleted, and the new code is different enough to defeat any `diff` program. That being said, I have done what I can to provide a line-for-line translation as best I can, in the parts of the code that matter. It may help to use tools to display the old code and new code side-by-size, in an ad-hoc fashion, and manually scrolling the versions together.
The old shell tests were written in a stylized and clever manner to reduce the verbiage of writing in shell script. The new tests are written in a differently stylized manner, using API methods to provide much of the same level of convenience.
Of the six tests, 5 have an obvious name translation (the hyphen is dropped from `Class-Path.sh` and `Class-Path2.sh`, and the cryptically named `wcMineField.sh` is renamed to the less cryptic `WildcardMineField.java`.
The supporting API in the new `Util.java` class is somewhat similar to the older `Util.sh` code, but is much less of a line-for-line translation.
Where possible, tools are invoked using the tool's `ToolProvider` API, as compared to exec-ing a separate process. A separate process is still always necessary for the main Java launcher, and is sometimes needed for Java ... when javac should be executed in a different directory, or with different env variables, or when using the "class path wildcard" feature.
On the use of `cleanup` methods... all the tests here call a `cleanup()` method as part of initial setup, and after the test is complete. Such cleanup is unnecessary beforehand when using jtreg (because jtreg guarantees an empty scratch directory when a test is run) and can be inconvenient afterwards, if you want to debug why a test-case has failed, although for sure it is arguably good to do if the test has passed. But any change would be a difference in the old and new code, and should arguably be done separately. So, the existence and use of `cleanup` methods is as before.
Some code was commented out in the original tests, because various options and system properties became unavailable in JDK 9. The commented out code is preserved, if only to provide visual anchors between the old and new versions. It may be desirable to eventually clean out the commented-out code.
The old code supported use of `${TESTTOOLVMOPTS}` and `${TESTVMOPTS}` although it is not clear how effective such support was, or how much it was used. The new code does _not_ support these environment variables, or their equivalent system properties. Given that the new code uses the `ToolProvider` API where possible, it is not clear how (or how easily) we could integrate support for those options, and whether it is worth the effort.
Needless to say, all the new tests pass. For positive test cases, that is definitely reassuring. For negative test cases, it is somewhat harder to verify that each new test case fails for the same reason that the old test case failed, not least because of the lack of info provided by the old tests. For this reason, it is assumed it is sufficient to have confidence in the translation of all the test cases.
Separately, there is a minor fix in ToolBox, to address an NPE. The fix is small enough and obvious enough to include here, as compared to having a separate JBS issue and PR.
-------------
Commit messages:
- JDK-8293877: Rewrite MineField test
Changes: https://git.openjdk.org/jdk/pull/10366/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=10366&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8293877
Stats: 3256 lines in 15 files changed: 1979 ins; 1276 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/10366.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/10366/head:pull/10366
PR: https://git.openjdk.org/jdk/pull/10366
More information about the compiler-dev
mailing list