Java 8 RFR 8022094: BigDecimal/CompareToTests and BigInteger/CompareToTests are incorrect

[Brian Burkhalter]

Hi Stuart,

On Aug 1, 2013, at 7:54 PM, Stuart Marks wrote:

> Looks good. Always a good idea to make sure that failures are reported properly. :-)

Indeed.

> It does look like the following cases are repeated, in both tests:
> 
>    {valueOf(Long.MIN_VALUE+1),          valueOf(Long.MAX_VALUE), MINUS_ONE},
>    {valueOf(Long.MIN_VALUE+1).negate(), valueOf(Long.MAX_VALUE), ZERO},
> 
> Is this right?

Yes. I actually did not add any tests or test cases to BigDecimal, only fixed the intended use of the ones present. What was happening is this. Each test case was of the form

	{a, b, expected}

where "expected" is the correct result of a.compareTo(b). Each test case was used like this:

	failures += compareToTest(a, b, expected);
	failures += compareToTest(a.negate(), b, -1);
	failures += compareToTest(a, b.negate(), 1);
	failures += compareToTest(a.negate(), b.negate(), -expected);

which could instead be written as one test

	failures += compareToTest(this, other, expected);

with four text cases

	{a, b, expected},
	{-a, b, -1},
	{a, -b, 1),
	{-a, -b, -expected}

which is patently incorrect for some of the test cases. What I did to fix this was to remove the two middle tests for each test case and add a new test case defined as

	{-a, b, expected2}

Another way to state this is that the original test case becomes two test cases

	{a, b, expected}
	{-a, b, expected2}

and the four tests become two

	failures += compareToTest(a, b, expected);
	failures += compareToTest(a.negate(), b.negate(), -expected);

So the test cases and tests are really the same as before, but the expected value is different in some instances as needed.

> The first four cases are (excuse the poor cross product notation):
> 
>    {max, max-1, min, min+1} x {1, -1} compareTo max
> 
> but then there's simply
> 
>    {min} x {1, -1} compareTo min
> 
> followed by the duplicate
> 
>    {min+1} x {1, -1} compareTo max
> 
> Just following the pattern, I'd expect
> 
>    {max, max-1, min, min+1} x {1, -1} compareTo min
> 
> though I'm not sure that all of these cases are necessary.

You are absolutely correct and I have updated the webrev accordingly:

http://cr.openjdk.java.net/~bpb/8022094/

Now only a Reviewer approval is needed.

Thanks,

Brian