RFR(S): JDK-8133180 - [TESTBUG] runtime/SharedArchiveFile/SharedStrings.java failed with WhiteBox.class : no such file or directory

[Christian Tornqvist]

Hi Misha,

In test/runtime/SharedArchiveFile/SharedStringsWb.java you changed the
String from "java" to "<init>", is this part of this change? If so, the
comment above should be corrected.

Otherwise this looks good.

Thanks,
Christian

-----Original Message-----
From: Mikhailo Seledtsov [mailto:mikhailo.seledtsov at oracle.com] 
Sent: Monday, August 31, 2015 7:02 PM
To: Jiangli Zhou <jiangli.zhou at oracle.com>; Ioi Lam <ioi.lam at oracle.com>;
Christian Törnqvist <Christian.Tornqvist at oracle.com>
Cc: hotspot-runtime-dev <hotspot-runtime-dev at openjdk.java.net>
Subject: Re: RFR(S): JDK-8133180 - [TESTBUG]
runtime/SharedArchiveFile/SharedStrings.java failed with WhiteBox.class : no
such file or directory

Jiangli,

  When you have a chance could you please run a final review on the last
change?
     http://cr.openjdk.java.net/~mseledtsov/8133180.01/

Also, could a Capital-R Reviewer review this change? (I believe I need a "R"
reviewer for this change)

Thank you,
Misha

On 8/28/2015 4:09 PM, Mikhailo Seledtsov wrote:
> Jiangli,
>
>  Thank you for a quick reply.
>
> I have updated the webrev accordingly:
>    http://cr.openjdk.java.net/~mseledtsov/8133180.01/
>
> Testing:
>     - ran the test with b75 (internal and openjdk)
>     - ran the "easy reproducer" discussed in a bug
>         rm -Rf JT* test
>         jtreg //hs-rt/hotspot/test/testlibrary_tests/ctw/JarDirTest.java
>         jtreg
> //hs-rt/hotspot/test/runtime/SharedArchiveFile/SharedStrings.java
>     - RBT of CDS tests - in progress
>
> Thank you,
> Misha
>
> On 8/28/2015 2:48 PM, Jiangli Zhou wrote:
>> Hi Misha,
>>
>> Your proposal sounds good.
>>
>> Thanks,
>> Jiangli
>>
>>> On Aug 28, 2015, at 12:38 PM, Mikhailo Seledtsov 
>>> <mikhailo.seledtsov at oracle.com> wrote:
>>>
>>> Hi Jiangli,
>>>
>>> I am back to this task after working on other stuff.
>>> As I started experimenting with your suggestions, I have found one 
>>> potential problem.
>>> The fallback strategy that you suggest could create an ambiguity.
>>>
>>> For instance, there may be cases where both 
>>> @build+ClassFileInstaller and @compile is used. The CFI will place 
>>> classes in the current working directory, aka ".", where is @compile 
>>> will place them under JTwork/../currentTest/classes/..
>>> In such cases, the test author will probably need to create 2 jars, 
>>> one for classes in the local directory, and another for classes in 
>>> the "@compile", and may want to be explicit what goes where, instead 
>>> of JarBuilder trying to guess and leading to possible ambiguity and 
>>> confusion.
>>>
>>> The key to solving this problem is to never assume the location of 
>>> classes produced by @build. If using @build, the classes MUST be 
>>> copied to the "." using ClassFileInstaller, since @build does not 
>>> guarantee the presence of these classes in a specific location.
>>> Perhaps, the best solution here is to require all classes used by 
>>> the JarBuilder to be in the "." directory, and require the test 
>>> author to use ClassFileInstaller when necessary. In such cases, if 
>>> the class is not in the test's current directory, the error will 
>>> become obvious immediately instead of under some occasional 
>>> conditions in the test system. It will also greatly simplify the 
>>> tests and will make them more robust.
>>>
>>> Please let me know comments/objections on such approach.
>>>
>>> Thank you,
>>> Misha
>>>
>>>
>>> On 8/17/2015 6:18 PM, Mikhailo Seledtsov wrote:
>>>> Hi Jiangli,
>>>>
>>>> Thank you for your suggestion; it should make testing more robust.
>>>> I will try your suggestion; if I do not see any undesired side 
>>>> effects I will rerun full testset and re-submit the updated review.
>>>>
>>>> Thank you,
>>>> Misha
>>>>
>>>> On 8/14/2015 5:43 PM, Jiangli Zhou wrote:
>>>>> Hi Misha,
>>>>>
>>>>> I have one suggestion. Instead of searching either the ‘work’ or 
>>>>> ‘classes’  directory depending on the ‘classesInWorkDir’ argument, 
>>>>> how about searching both directories? So the
>>>>> BasicJarBuilder.build() method would use the ‘classes’ directory 
>>>>> first, then try the ‘work’ directory if the file cannot be found 
>>>>> in ‘classes'. That might be a more robust solution.
>>>>>
>>>>> Thanks,
>>>>> Jiangli
>>>>>
>>>>> On Aug 14, 2015, at 2:22 PM, Mikhailo Seledtsov 
>>>>> <mikhailo.seledtsov at oracle.com> wrote:
>>>>>
>>>>>> Please review this fix to the CDS test bug. See the comments in 
>>>>>> the bug for details.
>>>>>>
>>>>>>     JBS: https://bugs.openjdk.java.net/browse/JDK-8133180
>>>>>>     Webrev: http://cr.openjdk.java.net/~mseledtsov/8133180.00/
>>>>>>     Testing:
>>>>>>         - ran the reproducer discussed in the bug description
>>>>>>             rm -Rf JT* test
>>>>>>             jtreg
>>>>>>
/media/data3/hg/9/work01/hs-rt/hotspot/test/testlibrary_tests/ctw/JarDirTest
.java
>>>>>>             jtreg
>>>>>> /media/data3/hg/9/work01/hs-rt/hotspot/test/runtime/SharedArchive
>>>>>> File/SharedStrings.java
>>>>>>
>>>>>>         - ran the CDS tests in concurrent mode
>>>>>>         - running CDS tests via multi-platform build-and-test system
>>>>>>             (in progress)
>>>>>>
>>>>>> Thank you,
>>>>>> Misha
>