[9] RFR JDK-8152344: JVMCI test task: Unit tests for ConstantReflectionProvider

Konstantin Shefov konstantin.shefov at oracle.com
Fri Mar 25 05:37:39 UTC 2016 

Christian

I have fixed "mx checkstyle" errors.
Then new webrev is: http://cr.openjdk.java.net/~kshefov/8152344/webrev.03/

-Konstantin

On 25.03.2016 03:00, Christian Thalinger wrote:
>
>> On Mar 24, 2016, at 11:37 AM, Konstantin Shefov 
>> <konstantin.shefov at oracle.com <mailto:konstantin.shefov at oracle.com>> 
>> wrote:
>>
>> Hi, Christian
>>
>> It seems my local ws has not built correctly after "hg tpull -u". I 
>> have cloned fresh WS, patched it and built it again.
>> Now "mx build --no-native" works with no errors.
>> Unit tests passed on Linux x64
>> Then I have done "mx ideinit" and opened project in IntelliJ Idea and 
>> reformatted all my files.
>>
>> Please look at the new version of the webrev: 
>> http://cr.openjdk.java.net/~kshefov/8152344/webrev.02/ 
>> <http://cr.openjdk.java.net/%7Ekshefov/8152344/webrev.02/>
>
> Excellent.  A couple things…
>
> I get 5 compiler warnings:
>
> /Users/cthaling/ws/jdk9/hs-comp/hotspot/test/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/TestHelper.java:474: 
> warning: [rawtypes] found raw type: Class
>     public static ResolvedJavaField getResolvedJavaField(Class clazz, 
> String fieldName) {
>                           ^
>   missing type arguments for generic class Class<T>
>   where T is a type-variable:
>     T extends Object declared in class Class
> /Users/cthaling/ws/jdk9/hs-comp/hotspot/test/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/BoxPrimitiveDataProvider.java:48: 
> warning: [cast] redundant cast to byte
>             cfgSet.add(new Object[]{JavaConstant.forByte((byte) number),
>                           ^
> /Users/cthaling/ws/jdk9/hs-comp/hotspot/test/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/ReadConstantArrayElementDataProvider.java:234: 
> warning: [rawtypes] found raw type: Class
>         Class dummyClass = DummyClass.class;
>         ^
>   missing type arguments for generic class Class<T>
>   where T is a type-variable:
>     T extends Object declared in class Class
> /Users/cthaling/ws/jdk9/hs-comp/hotspot/test/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/ReadConstantArrayElementDataProvider.java:244: 
> warning: [rawtypes] found raw type: Class
>         Class componentType = arrayField.getType().getComponentType();
>         ^
>   missing type arguments for generic class Class<T>
>   where T is a type-variable:
>     T extends Object declared in class Class
> 5 warnings
>
> Please fix them.
>
> It seems you didn’t run checkstyle because I’m still seeing a lot of 
> errors.
>
> Btw. the error when running unittest is fine:
>
> There was 1 failure:
> 1) 
> initializationError(jdk.vm.ci.hotspot.test.HotSpotConstantReflectionProviderTest)
> java.lang.Exception: No runnable methods
>
> There some work we have to do in mx.
>
>>
>> Thanks
>> -Konstantin
>>
>> On 24.03.2016 19:33, Christian Thalinger wrote:
>>>
>>>> On Mar 23, 2016, at 9:37 PM, Konstantin Shefov 
>>>> <konstantin.shefov at oracle.com 
>>>> <mailto:konstantin.shefov at oracle.com>> wrote:
>>>>
>>>> Hi, Christian
>>>>
>>>> I did as you suggested, also removed jdk.test.lib.Triple, but when 
>>>> I execute
>>>>
>>>> mx build --no-native
>>>>
>>>> I have an error
>>>>
>>>> /media/kshefov/OS/Users/kshefov/workspaces/jdk9-hs-comp/hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot.amd64/src/jdk/vm/ci/hotspot/amd64/AMD64HotSpotJVMCIBackendFactory.java:125: 
>>>> error: cannot find symbol
>>>>         if ((config.vmVersionFeatures & config.amd64SHA) != 0) {
>>>>                                               ^
>>>>   symbol:   variable amd64SHA
>>>>   location: variable config of type HotSpotVMConfig
>>>> /media/kshefov/OS/Users/kshefov/workspaces/jdk9-hs-comp/hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot.amd64/src/jdk/vm/ci/hotspot/amd64/AMD64HotSpotJVMCIBackendFactory.java:126: 
>>>> error: cannot find symbol
>>>>             features.add(AMD64.CPUFeature.SHA);
>>>>                                          ^
>>>>   symbol:   variable SHA
>>>>   location: class CPUFeature
>>>> 2 errors
>>>
>>> What JDK are you using to build?
>>>
>>>>
>>>> -Konstantin
>>>>
>>>> On 23.03.2016 22:07, Christian Thalinger wrote:
>>>>>
>>>>>> On Mar 23, 2016, at 7:57 AM, Christian Thalinger 
>>>>>> <christian.thalinger at oracle.com 
>>>>>> <mailto:christian.thalinger at oracle.com>> wrote:
>>>>>>
>>>>>> I need to get a change into mx.  Will get back to you.
>>>>>
>>>>> Alright.  I created an mx pull request for TestNG support:
>>>>>
>>>>> https://github.com/graalvm/mx/pull/102
>>>>>
>>>>> In the meantime you can clone my mx repository on github.  Add 
>>>>> this patch to your changes:
>>>>>
>>>>> diff -r 1e385207d9f2 .mx.jvmci/suite.py
>>>>> --- a/.mx.jvmci/suite.pyFri Mar 18 15:54:47 2016 +0100
>>>>> +++ b/.mx.jvmci/suite.pyWed Mar 23 08:52:35 2016 -1000
>>>>> @@ -175,6 +175,18 @@ suite = {
>>>>>        "workingSets" : "JVMCI",
>>>>>      },
>>>>>
>>>>> + "jdk.vm.ci.hotspot.test" : {
>>>>> + "subDir" : "test/compiler/jvmci",
>>>>> + "sourceDirs" : ["src"],
>>>>> + "dependencies" : [
>>>>> +   "mx:TESTNG",
>>>>> +   "jdk.vm.ci.hotspot",
>>>>> + ],
>>>>> + "checkstyle" : "jdk.vm.ci.services",
>>>>> + "javaCompliance" : "1.8",
>>>>> + "workingSets" : "API,JVMCI",
>>>>> +    },
>>>>> +
>>>>>      "jdk.vm.ci.hotspotvmconfig" : {
>>>>>        "subDir" : "src/jdk.vm.ci/share/classes",
>>>>>        "sourceDirs" : ["src"],
>>>>>
>>>>> and run mx checkstyle.  You will see a bunch of errors.  Fix 
>>>>> these.  Make sure JVMCI builds through mx:
>>>>>
>>>>> $ mx build --no-native
>>>>>
>>>>> The biggest problem I see is:
>>>>>
>>>>> /Users/cthaling/ws/jdk9/hs-comp/hotspot/test/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/ReadConstantArrayElementDataProvider.java:47: 
>>>>> error: package jdk.test.lib does not exist
>>>>> import jdk.test.lib.Triple;
>>>>>
>>>>> Not sure if it really makes sense to have that dependency.  After 
>>>>> the build succeeds you can run all unit tests via:
>>>>>
>>>>> $ mx unittest
>>>>>
>>>>> Once that works, either open all files in your favorite IDE (after 
>>>>> doing mx ideinit) and reformat the files or run:
>>>>>
>>>>> $ mx eclipseformat
>>>>>
>>>>>>
>>>>>>> On Mar 22, 2016, at 1:50 AM, Konstantin Shefov 
>>>>>>> <konstantin.shefov at oracle.com 
>>>>>>> <mailto:konstantin.shefov at oracle.com>> wrote:
>>>>>>>
>>>>>>> Hi, Christian
>>>>>>>
>>>>>>> Thanks for reviewing.
>>>>>>>
>>>>>>> Here is the new patch 
>>>>>>> http://cr.openjdk.java.net/~kshefov/8152344/webrev.01/
>>>>>>>
>>>>>>> I moved tests to the folder 
>>>>>>> hotspot/test/compiler/jvmci/jdk.vm.ci.hotspot.test and tho the 
>>>>>>> package jdk.vm.ci.hotspot.test. Also I have done some code 
>>>>>>> re-formatting.
>>>>>>>
>>>>>>> -Konstantin
>>>>>>>
>>>>>>> On 03/22/2016 12:20 AM, Christian Thalinger wrote:
>>>>>>>> Sorry but I have to bring this up again… there is so much 
>>>>>>>> line-breaking going on it’s ridiculous.  Especially in:
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~kshefov/8152344/webrev.00/test/compiler/jvmci/constantReflectionProviderTest/BoxPrimitiveDataProvider.java.html 
>>>>>>>> <http://cr.openjdk.java.net/%7Ekshefov/8152344/webrev.00/test/compiler/jvmci/constantReflectionProviderTest/BoxPrimitiveDataProvider.java.html>
>>>>>>>>
>>>>>>>> Or this:
>>>>>>>>   118         Assert.assertEquals(CONSTANT_REFLECTION_PROVIDER.readStableFieldValue(field,
>>>>>>>>   119                                                                               receiver,
>>>>>>>>   120                                                                               isDefStab),
>>>>>>>>   121                 expected, "Unexpected result:");
>>>>>>>>
>>>>>>>> Here is a suggestion.  Move your tests into the package 
>>>>>>>> jdk.vm.ci.hotspot.test under:
>>>>>>>>
>>>>>>>> hotspot/test/compiler/jvmci/jdk.vm.ci.hotspot.test
>>>>>>>>
>>>>>>>> Look at jdk.vm.ci.runtime.test as an example.  Then we 
>>>>>>>> add jdk.vm.ci.hotspot.test to the mx configuration file and let 
>>>>>>>> the predefined formatting rules take care of it.
>>>>>>>>
>>>>>>>> This would be my preferred way because then the tests are 
>>>>>>>> automatically imported into IDEs as well.  Actually all JVMCI 
>>>>>>>> tests should do it this way.
>>>>>>>>
>>>>>>>>> On Mar 21, 2016, at 11:07 AM, Konstantin Shefov 
>>>>>>>>> <konstantin.shefov at oracle.com> wrote:
>>>>>>>>>
>>>>>>>>> Hello
>>>>>>>>>
>>>>>>>>> Please review some new unit tests for methods of JVMCI 
>>>>>>>>> jdk.vm.ci.hotspot.HotSpotConstantReflectionProvider class.
>>>>>>>>>
>>>>>>>>> The tests cover most public methods of the class named above.
>>>>>>>>>
>>>>>>>>> Here 
>>>>>>>>> "test/compiler/jvmci/constantReflectionProviderTest/HotSpotConstantReflectionProviderTest.java" 
>>>>>>>>> is the JTREG-testng file, all other classes are testng data 
>>>>>>>>> providers, except TestHelper.java and DummyClass.java that 
>>>>>>>>> store all necessary variables.
>>>>>>>>>
>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8152344
>>>>>>>>> Webrev: http://cr.openjdk.java.net/~kshefov/8152344/webrev.00/
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>> -Konstantin
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160325/d21c813d/attachment-0001.html>

More information about the hotspot-compiler-dev mailing list