RFR: 8229186: Improve error messages for TestStringIntrinsics failures

[Evgeny Nikitin]

On Thu, 10 Sep 2020 12:20:05 GMT, Evgeny Nikitin <enikitin at openjdk.org> wrote:

> pre-Skara RFR thread: [link](https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-May/038416.html)
> 
> Error reporting was improved by writing a C-style escaped string representations for the variables passed to the
> methods being tested. For array comparisons, a dedicated diff-formatter was implemented.
> Sample output for comparing byte arrays (with artificial failure):
>  ----------System.err:(21/1553)----------
>  Result: (false) of 'arrayEqualsB' is not equal to expected (true)
>  Arrays differ starting from [index: 7]:
>  ... 5, 6,   7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, ...
>  ... 5, 6, 125, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, ...
>            ^^^^
>  java.lang.RuntimeException: Result: (false) of 'arrayEqualsB' is not
>  equal to expected (true)
>           at
>  compiler.intrinsics.string.TestStringIntrinsics.invokeAndCheckArrays(TestStringIntrinsics.java:273)
>            at ... stack trace continues - E.N.
>  Sample output for comparing char arrays:
>  ----------System.err:(21/1579)*----------
>  Result: (false) of 'arrayEqualsC' is not equal to expected (true)
>  Arrays differ starting from [index: 7]:
>  ... \\u0005, \\u0006, \\u0007, \\u0008, \\u0009, \\n, ...
>  ... \\u0005, \\u0006,      }, \\u0008, \\u0009, \\n, ...
>                      ^^^^^^^
>  java.lang.RuntimeException: Result: (false) of 'arrayEqualsC' is not
>  equal to expected (true)
>           at
>  compiler.intrinsics.string.TestStringIntrinsics.invokeAndCheckArrays(TestStringIntrinsics.java:280)
>            at
>  ... and so on - E.N.
> 
> testing: open/test/hotspot/jtreg/compiler/intrinsics/string/TestStringIntrinsics.java on linux, windows, macosx.

Responding to the comments from pre-Skara thread:

> test/hotspot/jtreg/compiler/intrinsics/string/TestStringIntrinsics.java:
>    I'd prefer invokeAndCompareArrays and invokeAndCheck to be as close as possible: have both of them to accept either
>    boolean or Object as 2nd arg; print/throw the same error message

the invokeAndCheck is very generic, it can be called with different objects and expect any kind of result, not only
boolean. Therefore its output format radically differs from what an array-comparator should show.

>  maybe I'm missing smth, but I don't understand why ArrayCodec supports only char and byte arrays; and hence I don't
>  understand why you need ArrayCodec::of methods, as you can simply do new
>  ArrayCoded(Arrays.stream(a).collect(Collectors.toList()) where a is an array of any type

for Object arrays, one can use that.
for integer primitives one needs Arrays.stream(a).boxed.collect(Collectors.toList()), please note 'boxed' - it is
required and not generic. for bytes or chars, there is none (no overload methos in the Arrays.stream(a));
To sum up, I can't see how with the given type system and utilities set I can make in a better, less wordy way. I've
added int and long overloads, support for String and Object arrays to make it more complete.

> it seems that ArrayCodec should be an inner static class of ArrayDiff

I would argue that - I find it useful for printing arrays (and this usage has been utilised in the
TestStringIntrinsics.java). In addition, I dont' like the practice of making such huge classes an inner classes as this
reduces readability and modularity.

Other issues have been fixed. I added support for int, long, Object and String arrays.

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

PR: https://git.openjdk.java.net/jdk/pull/112