RFR: 7903097: jtreg could implement ToolProvider [v3]

[Jonathan Gibbons]

On Mon, 14 Feb 2022 09:40:04 GMT, Christian Stein <cstein at openjdk.org> wrote:

>> Implement `ToolProvider`, now that the main `jtreg` tool requires JDK >= 9.
>
> Christian Stein has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Prevent non-zero exit code by using supported option

Better than before, you you can simplify the test even further, by invoking the test program directly (perhaps even with source launcher) instead of wrapping the test program in its own tiny jtreg test suite.

test/toolprovider/Pass.java line 29:

> 27:  * @test
> 28:  */
> 29: public class Pass

I would go the extra mile and make this a standalone program.  Not everything is a jtreg test,  especially tests designed to test jtreg ... that just adds a wrapper of confusion.

I would remove the `@test` comment, rename the class to something more like `ToolProviderTest.java` and invoke it directly from the `ToolProvider.gmk`, perhaps via source launcher, without going through the (unnecessary) extra layer of `jtreg`.

test/toolprovider/Pass.java line 36:

> 34: 
> 35:         int code = jtreg.run(System.out, System.err, "-help");
> 36: 

I would do a sanity check that at least something resembling command-line help output is generated.

test/toolprovider/ToolProviderTest.gmk line 26:

> 24: #
> 25: 
> 26: $(BUILDDIR)/ToolProviderTest.OK.ok: \

No need for the double `.OK.ok` suffix ... just `.ok` would be enough.

test/toolprovider/ToolProviderTest.gmk line 34:

> 32: 		-jdk:$(JDKHOME) \
> 33: 		$(TESTDIR)/toolprovider/Pass.java \
> 34: 			> $(@:%.ok=%/jt.log) 2>&1 ; rc=$$? ; \

It really doesn't add anything, other than an unnecessary level of complexity, to invoke the test using jtreg. And using jtreg to launch tests that run jtreg is just confusing.

It should be sufficient to replace these lines with a simple compile-and-run of the test program, perhaps even using source launcher.

test/toolprovider/ToolProviderTest.gmk line 40:

> 38: 
> 39: TESTS.jtreg += \
> 40: 	$(BUILDDIR)/ToolProviderTest.OK.ok

Double suffix again

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

PR: https://git.openjdk.java.net/jtreg/pull/54