[1145] Use the objectsToString() method to covert failure messages to…

[jamezp]

… a string rather than using toString() on the object array.

resolves #1145

I don't know if this is the correct branch. If not please let me know and I can migrate it elsewhere.

[spericas]

@alwin-joseph Could you take a look at this one?

[alwin-joseph]

Since we are considering a patch release 3.1.3-1 of the TCK with added LICENCE in jar file (#1142) , is it ok to have this change included in the same ?

@mkarg @jansupol @spericas

Otherwise this change looks fine to me.

[alwin-joseph]

I believe this change qualifies as "ehancement" as per https://jakarta.ee/committees/specification/tckprocess/.

Requests for improvement to tests MUST simply be created as issues with a label of enhancement in the specification project’s TCK issue tracker.

Hopefully this can be included in the next service release as the tests are not amended here.

[jamezp]

I believe this change qualifies as "ehancement" as per https://jakarta.ee/committees/specification/tckprocess/.

Requests for improvement to tests MUST simply be created as issues with a label of enhancement in the specification project’s TCK issue tracker.

Hopefully this can be included in the next service release as the tests are not amended here.

I'd actually disagree and consider it a bug. Right now the failure message looks something like:

[Ljava.lang.Object;@7de4bd8f ==> expected: <true> but was: <false>

This simply converts the object array into a readable string which seems to be the original intention and was just missed. The objectsToString() method was already there and the client is just invoking Object[].toString() which isn't useful.

[jansupol]

I'd actually disagree and consider it a bug.

if it were qualified as a bug, an exclusion would need to be created with a description of why it makes the test fail. Considering it an enhancement request, the process would be much easier, just merge the PR and have it nicely in the next release. No?

[jamezp]

if it were qualified as a bug, an exclusion would need to be created with a description of why it makes the test fail. Considering it an enhancement request, the process would be much easier, just merge the PR and have it nicely in the next release. No?

I guess it depends on how you look at it. The failure message is really not useful at all which IMO is a bug. The test itself is not wrong, it's just the generated message is useless. I'm fine with having this fixed in 4.0, we're past 3.1 at this point for the most part :)

[alwin-joseph]

If you agree we can redirect this change to master branch for 4.0 release then.

[jamezp]

@alwin-joseph Sure thing, I just changed the branch to release-4.0.

[spericas]

LGTM

[jamezp]

Can this be merged or do we need some changes? The messages printed from the TCK aren't much good. You've got to debug a TCK test to see what the message is. For example:

assertNotNull(holder.get(), "Message not received, reconnect was done",
    cnt - 1, "times.");

Prints the following without the change:

[ERROR]   JAXRSClientIT.connectionLostFor1500msTest:591->JAXRSCommonClient.assertNotNull:768 [Ljava.lang.Object;@4674d90 ==> expected: <true> but was: <false>

The message is fairly useless.

[alwin-joseph]

Based on the previous discussion this is considered for 4.0 release.
+1 for merging.