RFR(XS) : 8186095 : upgrade to jtreg 4.2 b08

[David Holmes]

Thanks Ioi, and Igor, for clarifying.

David


On 11/08/2017 3:23 PM, Ioi Lam wrote:
> 
> 
> On 8/10/17 9:46 PM, David Holmes wrote:
>> On 11/08/2017 2:31 PM, Igor Ignatyev wrote:
>>>
>>>> On Aug 10, 2017, at 9:22 PM, David Holmes <david.holmes at oracle.com> 
>>>> wrote:
>>>>
>>>> Hi Igor,
>>>>
>>>> On 11/08/2017 2:02 PM, Igor Ignatyev wrote:
>>>>> http://cr.openjdk.java.net/~iignatyev//8186095/webrev.00/index.html
>>>>>> 14 lines changed: 1 ins; 0 del; 13 mod;
>>>>> Hi all,
>>>>> could you please review this small patch which bumps up jtreg version?
>>>>> besides updating jib profiles and all TEST.ROOT files,
>>>>
>>>> That all looks fine (though deploy should not be in there).
>>>>
>>>>> the fix updates
>>>>> hotspot/test/runtime/Metaspace/FragmentMetaspaceSimple.java test 
>>>>> not to
>>>>> rely on having "library" test.Empty class in 'test.classes' and put
>>>>> test.Empty class in the workdir instead.
>>>>
>>>> Sorry I'm not following this part. You made two changes:
>>>>
>>>> 1. Added @library /test/lib
>>> /test/lib is needed for ClassFileInstaller.
>>
>> Okay.
>>
>>>>
>>>> What is this doing? (For that matter what is the existing "classes" 
>>>> entry supposed to mean ??? how is "classes" a library?)
>>> existing 'classes' is the directory in 
>>> hotspot/test/runtime/Metaspace/ which contains source of test.Empty.
>>
>> Okay.
>>
>>>>
>>>> 2. Instead of the test reading from test.classes you are using the 
>>>> ClassfileInstaller to copy the class to the working directory.
>>>>
>>>> How does this make a difference to anything? If the test wouldn't 
>>>> find the class in test.classes, doesn't that mean ClassfileInstaller 
>>>> will also fail to find it?
>>> test.classes points to the directory w/ classes from a test, but not 
>>> from test libraries. directories w/ all needed classes (either from a 
>>> test or from libraries) are added to classpath and 'test.class.path'. 
>>> ClassFileInstaller uses class loader to get resources, test.Empty 
>>> will be in CP, so ClassFileInstaller will have access to it.
>>
>> Sorry still don't understand the change. Where does:
>>
>> @build test.Empty
>>
>> place Empty.class? If not in test.classes then how has this test ever 
>> passed? I'm assuming the change is needed because it no longer passes 
>> with the updated jtreg.
>>
> Hi David,
> 
>    27  * @build test.Empty
> 
> puts test.Empty class somewhere in the classpath
> 
>    28  * @run driver ClassFileInstaller test.Empty
> 
> ClassFileInstaller uses Class.getResourceAsStream("test/Empty.class") to 
> read the contents of this class (from the CLASSPATH), and writes it to 
> ./test/Empty.class
> 
>    55         String fileName = "test" + sep + "Empty.class";
>    56         File file = new File(fileName);
>    57         byte buff[] = read(file);
> 
> here the test reads "./test/Empty.class" into a buffer. (When the test 
> runs, the current directory is the "scratch" directory).
> 
> The old version of this test assumed that JTREG builds the Empty.class 
> into a certain location, but that has changed with the jtreg 4.2 b08 
> feature.
> 
> ClassFileInstaller is the proper way of reading the bytes of class 
> files, and has been used by most other test cases. 
> FragmentMetaspaceSimple seems to be the only exception.
> 
> Thanks
> - Ioi
> 
> 
> 
> 
>> Thanks,
>> David
>>
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>> testing: :hotspot_all, {jdk,langtools,nashorn,jaxp}/test/:tier[1-3]
>>>>> Thanks,
>>>>> -- Igor
>>>
>