Review request: JDK-8006753 fix failed for JDK-8002415 White box testing API for HotSpot

[Vladimir Kozlov]

Changes are good for push.

I only worried about the first issue but I reproduced your case with 
next instruction:

$ hg rename src/share/tools/whitebox test/testlibrary/whitebox
moving src/share/tools/whitebox/sun/hotspot/WhiteBox.java to 
test/testlibrary/whitebox/sun/hotspot/WhiteBox.java
moving 
src/share/tools/whitebox/sun/hotspot/parser/DiagnosticCommand.java to 
test/testlibrary/whitebox/sun/hotspot/parser/DiagnosticCommand.java

Did your 'hg st' shows the next?:

$ hg st
A test/testlibrary/whitebox/sun/hotspot/WhiteBox.java
A test/testlibrary/whitebox/sun/hotspot/parser/DiagnosticCommand.java
R src/share/tools/whitebox/sun/hotspot/WhiteBox.java
R src/share/tools/whitebox/sun/hotspot/parser/DiagnosticCommand.java

Thanks,
Vladimir

On 2/19/13 11:21 AM, Mikael Gerdin wrote:
> Vladimir,
>
> On 2013-02-19 20:18, Vladimir Kozlov wrote:
>> Mikael,
>>
>> Did you use 'hg rename' to move WhiteBox.java and
>> DiagnosticCommand.java? Or you just copied it? I don't see in
>> hsx-rt.patch that they were removed from src/share/tools/whitebox.
>
> Yes, if you look at the webrev html it says
> " test/testlibrary/whitebox/sun/hotspot/WhiteBox.java (was
> src/share/tools/whitebox/sun/hotspot/WhiteBox.java"
> but you are correct in that the hsx-rt.patch file does not contain that
> information, but the changeset in my local repository does have that
> information. I believe this is because webrev does not create a "real"
> hg patch with all the releavant hg metadata.
>
>>
>> I noticed that changeset's Summary still reference ClassFileDumper:
>
> Oops, will fix that.
>
>>
>> Summary: Modify WhiteBoxAPI to use interface classes from
>> test/testlibrary instead, add ClassFileDumper to resolve the boot class
>> path issue
>
> Given that fix, are you ok with me pushing this?
>
> /Mikael
>
>>
>> Thanks,
>> Vladimir
>>
>> On 2/19/13 10:15 AM, Mikael Gerdin wrote:
>>> Vladimir, all
>>>
>>> I've merged the recent whitebox test additions into my change and added
>>> the missing copyright headers.
>>> Here's the latest and greatest webrev:
>>> http://cr.openjdk.java.net/~mgerdin/8006753/webrev.2/
>>>
>>> I've also ran these tests with jprt -retest to see that the new variant
>>> works on different platforms.
>>>
>>> When doing the change to PrintNMTStatistics.java I discovered that the
>>> test was in fact only compiled with javac and not run, and in fact
>>> needed further updates to work properly. I filed a bug for that test and
>>> updated it to compile the java sources using the new scheme.
>>>
>>> /Mikael
>>>
>>> On 02/06/2013 07:55 PM, Vladimir Kozlov wrote:
>>>> On 2/6/13 10:16 AM, Mikael Gerdin wrote:
>>>>> Vladimir,
>>>>>
>>>>> On 2013-02-06 17:30, Vladimir Kozlov wrote:
>>>>>> Mikael,
>>>>>>
>>>>>> I like that all WB functionality is moved to test directory.
>>>>>>
>>>>>> You missed Copyright header in all new files.
>>>>>
>>>>> Thanks for the reminder, I'm not used to creating new files :)
>>>>
>>>> I still don't see it in DiagnosticCommand.java. Also old files
>>>> WBApi.java and ParserTest.java do not have it.
>>>>
>>>> Mikael, we need Copyright header in all files we push into open
>>>> sources.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>>>
>>>>>>
>>>>>> Could you rename ClassFileDumper to ClassFileInstaller? It will be
>>>>>> more
>>>>>> clear what it does.
>>>>>
>>>>> Done. Updated webrev at:
>>>>> http://cr.openjdk.java.net/~mgerdin/8006753/webrev.1/
>>>>>
>>>>> /Mikael
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>>
>>>>>> On 2/4/13 5:59 AM, Mikael Gerdin wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Background:
>>>>>>> Even from the beginning of the WhiteBox testing API implementation
>>>>>>> the
>>>>>>> tests have not worked on builds promoted by release engineering
>>>>>>> due to
>>>>>>> the fact that the wb.jar file containing the Java classes was not
>>>>>>> imported by the JDK-level makefiles.
>>>>>>> On one level this was ok since we didn't want to ship the classes to
>>>>>>> end
>>>>>>> users and we worked around the complexity of somehow post-processing
>>>>>>> the
>>>>>>> RE bundles to remove them.
>>>>>>>
>>>>>>> The problem is that we aren't able to run the WhiteBox tests in
>>>>>>> promotion testing, or to verify fixes against earlier builds.
>>>>>>>
>>>>>>> Solution:
>>>>>>> The solution suggested here is to put the WhiteBox helper classes in
>>>>>>> the
>>>>>>> testlibrary directory under test/ and
>>>>>>> * tell jtreg to first compile the classes
>>>>>>> * run a special Java program to copy the classes to the test work
>>>>>>> directory
>>>>>>> * run the white box test with -Xbootclasspath/a:. to pick up the
>>>>>>> copied
>>>>>>> classes.
>>>>>>>
>>>>>>> The solution is kind-of hackish but I've been unable to come up
>>>>>>> with a
>>>>>>> better solution without performing complex surgery on jtreg.
>>>>>>>
>>>>>>> The changes to the make/ directory should basically revert the
>>>>>>> makefile
>>>>>>> changes that were added to support this API.
>>>>>>>
>>>>>>> Webrev:
>>>>>>> http://cr.openjdk.java.net/~mgerdin/8006753/webrev.0/
>>>>>>> Bug:
>>>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8006753
>>>>>>>
>>>>>>> Note about renamed/moved files:
>>>>>>> I will try to make sure that the moved/renamed files will be tracked
>>>>>>> properly by mercurial, but I think that using patch files and MQ
>>>>>>> breaks
>>>>>>> this so I'll do this after the review is done.
>>>>>>>
>>>>>>> Thanks
>>>>>>> /Mikael
>>>