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

[Vladimir Kozlov]

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.

I noticed that changeset's Summary still reference ClassFileDumper:

Summary: Modify WhiteBoxAPI to use interface classes from 
test/testlibrary instead, add ClassFileDumper to resolve the boot class 
path issue

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
>