Review request: JDK-8004928 TEST_BUG: Reduce dependence of CoreLib tests from the AWT subsystem (ver. 2)

Alexey Utkin alexey.utkin at
Thu Dec 13 11:48:47 UTC 2012

Here is new version fix:

On 12.12.2012 22:59, Mandy Chung wrote:
> On 12/12/12 7:41 AM, Alexey Utkin wrote:
>> Bug description:
>> Summary:
>> *test/java/io/Serializable/resolveProxyClass/*
>> *test/java/lang/reflect/Proxy/*
>>         The set of non-public interfaces was changed to avoid AWT 
>> dependencies.
> These 2 tests look for the first non-public system interface.  Instead 
> of keeping the nonPublicInterfaces list, it may be good to simplify 
> this to use "" which seems to be a stable 
> one that will unlikely be removed.  I would suggest to add a check 
> that the class is non-public (Modifier.isPublic) to catch any future 
> change to this class.

>>         Swing-stored constants were changed to locally-defined.
> *test/java/util/Collections/*
> I believe your change captures what it's intended to test (typed 
> enumeration and rawtype enumeration).  Minor comment - it might be 
> better to move the new static variables as local variable before 
> calling testEmptyEnumeration to be consistent with convention used in 
> this test.  There is no need to keep them static anyway.   I would 
> also take out the comment about the substitution since this test 
> doesn't seem to have any relationship with javax.swing.

>> *test/java/util/logging/*
>>         Test case was simplified to avoid AWT class loading. Negative 
>> test result was tested on early
>>         JDK7 build.
> Looks good.  Nit: L46 - good to break it into 2 lines.  It's good that 
> you have verified with and without the fix.

On 12.12.2012 22:40, Alan Bateman wrote:
> For java/io/Serializable/resolveProxyClass/ and 
> java/lang/reflect/Proxy/ then it would be nice 
> if the types used were in compact1 [1], that would avoid needing to 
> exclude those tests. I also see the test uses 
> which I don't think exists but perhaps 
> that is intentional.
The list was reduces to "" (from compact1 
profile) with "non-public" status verification.

> test/java/util/Collections/, minor nit but I think 
> we prefer "public static" over "static public". It doesn't of course 
> need to be public anyway.
The variables were made local-final in accordance with Mandy's 

> You probably saw Dan's comment about changing 
> test/java/util/logging/, I trust you'll double 
> check this test with an older version of the JDK that doesn't have the 
> fix. My only comment is that line 46 is too wide.

Thanks for review!

More information about the core-libs-dev mailing list