RFR(S) : 8181053 : port basicvmtest to jtreg

[Erik Helin]

On 06/27/2017 04:48 PM, Igor Ignatyev wrote:
> Hi Erik,
>
> I have mentioned this in my 1st email. anyhow, changing the product and the associated potential race are the exact reasons why it was decided not to port this test. I have checked w/ Misha and he assured me we have other tests for -Xshare:dump. so answering your question, scrap this -Xshare:dump sanity test in favor of existing jtreg tests, e.g. runtime/SharedArchiveFile.

*sigh*, sorry, I forgot you explained this in the first email. Sounds 
good then, now go ahead and push this :)

Erik

> thank you one more time for your review.
>
> Cheers,
> -- Igor
>> On Jun 26, 2017, at 11:45 PM, Erik Helin <erik.helin at oracle.com> wrote:
>>
>> Hi Igor,
>>
>> looking at this one extra time, I realized that the following change to
>> hotspot/test/Makefile:
>>
>> -# clienttest (make sure various basic java client options work)
>> -
>> -hotspot_clienttest clienttest: sanitytest
>> -        $(RM) $(PRODUCT_HOME)/jre/lib/*/client/classes.jsa
>> -        $(RM) $(PRODUCT_HOME)/jre/bin/client/classes.jsa
>> -        $(PRODUCT_HOME)/bin/java $(JAVA_OPTIONS) -Xshare:dump
>> -
>> -PHONY_LIST += hotspot_clienttest clienttest
>>
>> actually removes one additional test that isn't covered by your newly
>> added file hotspot/test/sanity/BasicVMTest.java: sanity testing
>> -Xshare:dump.
>>
>> Do we want to add a small test in hotspot/test/sanity for -Xshare:dump?
>> Or is this functionality tested elsewhere? A related question: if
>> multiple tests were running concurrently (testing the same JDK), won't
>> there be a race condition with the above test? For example if two JTReg
>> tests are running (and -conc > 1) and both JTReg tests tries to remove
>> classes.jsa and then regenerate them, seems like there could be a race?
>>
>> What do you think? Just scrap the -Xshare:dump sanity test or add a
>> JTReg version?
>>
>> Thanks,
>> Erik
>>
>> On 06/14/2017 09:41 AM, Erik Helin wrote:
>>> On 06/14/2017 01:09 AM, Igor Ignatyev wrote:
>>>> http://cr.openjdk.java.net/~iignatyev//8181053/webrev.00/index.html
>>>>> 121 lines changed: 54 ins; 67 del; 0 mod;
>>>>
>>>> Hi all,
>>>>
>>>> could you please review this small patch which introduces jtreg
>>>> version of basicvmtest test?
>>>>
>>>> make version of basicvmtest also included sanity testing for CDS on
>>>> client JVM, but this testing modified the product binaries, so it
>>>> might interfere with results of other tests and is not very reliable.
>>>> I have consulted w/ Misha about this, and he assured me that there are
>>>> other better CDS tests which check the same functionality, so we
>>>> should not lose test coverage there.
>>>>
>>>> webrev:
>>>> http://cr.openjdk.java.net/~iignatyev//8181053/webrev.00/index.html
>>>
>>> Looks good, Reviewed.
>>>
>>> Thank you for this patch Igor! I've been meaning to fix this for a long
>>> time but never got around to it...
>>>
>>> Erik
>>>
>>>> jbs: https://bugs.openjdk.java.net/browse/JDK-8181053
>>>> testing: jprt, new added test
>>>>
>>>> Thanks,
>>>> -- Igor
>>>>
>