RFR: 8229186: Improve error messages for TestStringIntrinsics failures

[Igor Ignatyev]

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

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

I am not sure I understand what you mean...
1. granted you can't change `invokeAndCheck`'s 2nd argument to bool as there are other values being passed, but you can
change  `invokeAndCompareArrays` to accept an `Object` and compare expected and actual values by `Object::equals`. 2.
even if you can't change output of these two methods to be the same (which I so far failed to see why), you still can
change `invokeAndCheck`'s `message` var to include actual and expected values in the same way as
`invokeAndCompareArrays` does.

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

you don't need `ArrayCodec::of(Object array)` anymore, do you?

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

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

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

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