Re: RFR: 8295239: Refactor java/util/Formatter/Basic script into a Java native test launcher

[Lance Andersen]

On Fri, 14 Oct 2022 20:38:32 GMT, Justin Lu <d...@openjdk.org> wrote:

> Issue: Formatter unit tests are launched via basic.sh
> 
> Fix: Replace basic.sh with a Java test launcher
> 
> Note: Java.internal.math was included in the original configuration of Basic, 
> but I removed it as it was not used within the Basic unit tests 
> 
> 
> Original output on success
> <img 
> src="https://user-images.githubusercontent.com/67398801/195936541-bc90db50-8d03-47be-9c4f-95176b19a6a7.png";
>  width="350" height="350">
> 
> 
> New output on success
> <img 
> src="https://user-images.githubusercontent.com/67398801/195936558-f85f4d48-dae2-4c38-aa50-46ef47db3d89.png";
>  width="350" height="450">

Hi Justin,

A few comments on a quick pass through your changes

test/jdk/java/util/Formatter/Basic.java line 93:

> 91:                                        + fail + " failure(s), first", 
> first);
> 92:         else
> 93:             System.out.println("all " + (fail + pass) + " tests passed");

Perhaps use System.out.printf vs println and "fail" should not be needed as all 
tests passed

test/jdk/java/util/Formatter/BasicTestLauncher.java line 52:

> 50: 
> 51:     @Test
> 52:     public void testUsPac() throws IOException{

You could use a DataProvider

test/jdk/java/util/Formatter/BasicTestLauncher.java line 99:

> 97:         }catch(RuntimeException err){
> 98:             throw new RuntimeException(String.format("$$$ %s: Test(s) 
> failed or TestJVM did not build correctly." +
> 99:                     " Check stderr output from diagnostics summary 
> above%n",  err.getMessage()));

Do you need to catch the RuntimeException and then throw a new 
RuntimeException?  I think you might want to consider reworking this

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

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