[OpenJDK 2D-Dev] Review Request for JDK-7107905: ColorModel subclasses are missing hashCode() or equals() or both methods

Jim Graham james.graham at oracle.com
Mon Jan 30 21:40:15 UTC 2017

Hmmm.  Sounds like the test cases were written based on bugs in the implementation.  I'm not sure what the best tactic 
is here for the short term for getting this in, but many of these changes should eventually be considered bugs in the 
tests.  Is it acceptable to break API tests like this at the last minute even if the tests are at fault?  Phil?

Notes on specific instances below...

On 1/30/17 2:22 AM, Jayathirth D V wrote:
> Hi Phil,
> 	1)api/java_awt/Image/ColorModel/index.html#Constructor: Failed. test cases: 4; passed: 3; failed: 1; first test case failure: ColorModel2001
> 	This test fails because getComponentSize() returned an array with length 3 but it expects the length to be 4. In the test case they have bits per component array 	of length 4 like {8, 8, 8, 8}. But in the test case wherever they are passing "has Alpha" as "false" we omit the alpha component bit. This is because of tighter check 	that we have in ColorModel class as "nBits = Arrays.copyOf(bits, numComponents);" . "numComponents" will be 3 if hasAlpha is false.

This is a bug in the test then, especially if the size of our array matches the return value of getNumComponents.

> 	2)api/java_awt/Image/ColorModel/index.html#Equals: Failed. test cases: 3; passed: 2; failed: 1; first test case failure: ColorModel0004
> 	Here they check for equality between 2 ColorModel objects having same values, but it fails because now we are using identity-as-equals check in ColorModel.

How do they accomplish this when the CM class is abstract?  Do they create a relatively empty subclass and instantiate that?

The documentation for the equals() method does not document the conditions under which it returns true, it uses a vague 
concept of "equals this ColorModel".  I don't see how they could test anything given that documentation.

> 	3)api/java_awt/Image/ColorModel/index.html#HashCode: Failed. test cases: 2; passed: 1; failed: 1; first test case failure: ColorModel2006
> 	Here they check for hashCode equality between 2 ColorModel objects having same values, but it fails since we don't have hashCode check in ColorModel and it 	will be different between 2 Objects.

Same as above, there are no promises documented.

> 	4)api/java_awt/Image/ComponentColorModel/index.html#ConstructorTesttestCase1: Failed. test cases: 2; passed: 1; failed: 1; first test case failure: testCase1
> 	Throws "java.lang.ArrayIndexOutOfBoundsException: 3". This is also happening because of same reason as why the first JCK test is failing. We omit alpha bit if 	hasAlpha is false but JCK test tries to call getComponentSize() with index 3 which throws ArrayIndexOutOfBoundsException.

Same assessment as #1 above...

Again, while these are my recommendations about the correctness of these tests, the question remains whether we want to 
introduce a change at this point in the release cycle that will essentially invalidate a number of tests that have been 
working for several releases already.  I'll leave that tactic issue to Phil...


More information about the 2d-dev mailing list