[9] RFR JDK-8152344: JVMCI test task: Unit tests for ConstantReflectionProvider
Christian Thalinger
christian.thalinger at oracle.com
Fri Mar 25 06:47:27 UTC 2016
> On Mar 24, 2016, at 7:37 PM, Konstantin Shefov <konstantin.shefov at oracle.com> wrote:
>
> Christian
>
> I have fixed "mx checkstyle" errors.
> Then new webrev is: http://cr.openjdk.java.net/~kshefov/8152344/webrev.03/ <http://cr.openjdk.java.net/%7Ekshefov/8152344/webrev.03/>
Looks good now. Thanks for making all the changes I requested. Reviewed.
Since I don’t have time right now to add mx TestNG support could you add this to your patch?
diff -r 1e385207d9f2 .mx.jvmci/suite.py
--- a/.mx.jvmci/suite.py Fri Mar 18 15:54:47 2016 +0100
+++ b/.mx.jvmci/suite.py Thu Mar 24 20:46:17 2016 -1000
@@ -32,24 +32,9 @@ suite = {
"libraries" : {
- "HCFDIS" : {
- "urls" : ["https://lafo.ssw.uni-linz.ac.at/pub/hcfdis-3.jar"],
- "sha1" : "a71247c6ddb90aad4abf7c77e501acc60674ef57",
- },
-
- "C1VISUALIZER_DIST" : {
- "urls" : ["https://java.net/downloads/c1visualizer/c1visualizer_2015-07-22.zip"],
- "sha1" : "7ead6b2f7ed4643ef4d3343a5562e3d3f39564ac",
- },
-
- "JOL_INTERNALS" : {
- "urls" : ["https://lafo.ssw.uni-linz.ac.at/pub/truffle/jol/jol-internals.jar"],
- "sha1" : "508bcd26a4d7c4c44048990c6ea789a3b11a62dc",
- },
-
- "BATIK" : {
- "sha1" : "122b87ca88e41a415cf8b523fd3d03b4325134a3",
- "urls" : ["https://lafo.ssw.uni-linz.ac.at/pub/graal-external-deps/batik-all-1.7.jar"],
+ "TESTNG" : {
+ "urls" : ["http://central.maven.org/maven2/org/testng/testng/6.9.10/testng-6.9.10.jar"],
+ "sha1" : "6feb3e964aeb7097aff30c372aac3ec0f8d87ede",
},
# Stubs for classes introduced in JDK9 that allow compilation with a JDK8 javac and Eclipse.
@@ -175,6 +160,18 @@ suite = {
"workingSets" : "JVMCI",
},
+ "jdk.vm.ci.hotspot.test" : {
+ "subDir" : "test/compiler/jvmci",
+ "sourceDirs" : ["src"],
+ "dependencies" : [
+ "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"],
>
> -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 <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.py Fri Mar 18 15:54:47 2016 +0100
>>>>>> +++ b/.mx.jvmci/suite.py Wed 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/ <http://cr.openjdk.java.net/%7Ekshefov/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> <mailto: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 <https://bugs.openjdk.java.net/browse/JDK-8152344>
>>>>>>>>>> Webrev: http://cr.openjdk.java.net/~kshefov/8152344/webrev.00/ <http://cr.openjdk.java.net/%7Ekshefov/8152344/webrev.00/>
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> -Konstantin
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160324/5c467664/attachment-0001.html>
More information about the hotspot-compiler-dev
mailing list