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

[Mikael Gerdin]

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
>>