RFR: JDK-8293877: Rewrite MineField test

[Martin Buchholz]

On Wed, 21 Sep 2022 00:57:22 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:

> 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.

Explicit OS command invocation and ToolProvider invocation both seem important to test.
IIRC there were times when they were not in fact equivalent in effect.
Consider library infrastructure to run both variants and ensure results are identical.

I recall being slightly proud when writing the first iteration of this test - it allowed a very terse test description in shell.
But terseness is not universally admired, and even I (in my dotage) have joined the camp of writing tests in more verbose junit style.

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

PR: https://git.openjdk.org/jdk/pull/10366