RFR: 8214799: Add package declaration to each JTREG test case in the gc folder

[Leo Korinth]

Hi!

On 21/12/2018 21:50, Leonid Mesnik wrote:
> Hi
> 
> Great change. The only a couple of comments
> 
> 1) I see that you added a lot of relative links to include test-root as 
> library like:
> 
> 32  * @library ../../..
> 
> 
> I think you might want to use
>   @library  > or add to existing library test/lib like
> @library /test/lib /
> 
> instead. You might find a lot of examples in existing compiler and gc tests.
> 
> It is the more common an unified approach  rather then using relative 
> paths. Also it allows you to don't care when you move tests in different 
> packages.
> 
I will try that, it is certainly better. I think I will use a new 
@library line though, it is easier to see the change.
> 
> 2) New test in 
> http://cr.openjdk.java.net/~lkorinth/8214799/01/test/hotspot/jtreg/gc/epsilon/CriticalNativeStress.new.java.html
> looks confusing to me. It is copied from 
> http://cr.openjdk.java.net/~lkorinth/8214799/01/test/hotspot/jtreg/gc/stress/CriticalNativeStress.new.java.html
> 
> So now we have test CriticalNativeStress 
> gc/epsilon/CriticalNativeStress.java and CriticalNativeStressEpsilon in 
> gc/stress/CriticalNativeStress.java
> The code in both files and jtreg tags looks pretty similar. Have you 
> added it intentionally? Could you please explain a little bit purpose of 
> this.

Many thanks for finding this fault of mine. The file was moved with the 
Shenandoah push, and I must have rebased it badly although the move of 
the corresponding c-file went through, strange...

> Other changes looks good.
> 

Thanks for taking a look at it!
I will create a new webrev after the holiday.

/Leo

> Leonid


>> On Dec 21, 2018, at 11:32 AM, Leo Korinth <leo.korinth at oracle.com 
>> <mailto:leo.korinth at oracle.com>> wrote:
>>
>> Hi again!
>>
>> I am adding Leonid and Erik to the discussion to see if my package 
>> additions are in any way troublesome to the bigger picture. I believe 
>> they are not. We are already using Java packages in some tests; this 
>> change will just add them to more tests so that one can easily use an IDE.
>>
>> Please ignore the old webrev as I had to rebase against the resent 
>> Shenandoah push. It is easier to just read this version 
>> (http://cr.openjdk.java.net/~lkorinth/8214799/01/). Unfortunately no 
>> incremental version is given as this is a rebase.
>>
>> I have not had the time to update the test cases under the 
>> gc/shenandoah folder (I am also afraid to make this change any 
>> bigger), but I have tried to update the test cases that changed or was 
>> added by the Shenandoah push (some Epsilon and Shenandoah tests 
>> outside the gc/shenandoah folder).
>>
>> This rebase basically applies the same kind of changes to the added 
>> test cases. One change that I would like you to look more carefully at 
>> is the addition of a helper class CriticalNative.java that uses JNI. 
>> Without this refactoring I could not have used the same c-file as it 
>> was used in different packages (it is used from three different test 
>> cases, and the package is part of the JNI export name). Please take an 
>> extra look at that change.
>>
>> New webrev:
>> http://cr.openjdk.java.net/~lkorinth/8214799/01/
>>
>> Testing:
>> I have run jtreg tests under gc/epsilon and I have run :hotspot_gc 
>> with Shenandoah in order not to break those test cases.
>>
>> Also started new run of tier1-3 and :hotspot_gc
>>
>> Thanks,
>> Leo
>>
>> On 05/12/2018 14:44, Leo Korinth wrote:
>>> Hi,
>>> I have added package declarations to all jtreg tests in the gc folder 
>>> (and sub folders). Previously most tests were not in a package 
>>> (though some where), making it extremely hard to work with jtreg 
>>> tests in an IDE.
>>> The main change consists of adding a package declaration that 
>>> corresponds to the directory the test is located in. This also makes 
>>> it necessary to change @run annotations. Some test cases uses 
>>> "common" code that is not located in the test lib. Those test cases 
>>> now need to have a @library annotation that references the jtreg test 
>>> root.
>>> A few test cases had package visible classes with the same name in 
>>> the same directory. That did work before, but does not work anymore, 
>>> so I have prefixed those package visible classes with the public 
>>> class name.
>>> class TestVerifySilentlyRunSystemGC
>>> class TestVerifySubSetRunSystemGC
>>> class TestEagerReclaimHumongousRegionsReclaimRegionFast
>>> class TestEagerReclaimHumongousRegionsClearMarkBitsReclaimRegionFast
>>> class TestEagerReclaimHumongousRegionsWithRefsReclaimRegionFast
>>> Where classes have been referenced using string literals, I have 
>>> tried to use a .class.getName() instead (where possible).
>>> I have tried to update copyrights to all files (also adding a missing 
>>> comma after the second year in some files), and I did add missing 
>>> copyright to TestFromCardCacheIndex.
>>> known cons:
>>> - needs a package line in each java file
>>> - needs an extra library line in the test if the test is using other 
>>> files.
>>> - sometimes needs a run tag
>>> improvements:
>>> + can use eclipse or other IDEs without creating a workspace per test 
>>> (I guess it will be a significant improvement for users of emacs and 
>>> vi as well)
>>> + can use test files located in other directories
>>> After this change, I have several big cleanups that are easy to do 
>>> with a working IDE.
>>> Enhancement:
>>> https://bugs.openjdk.java.net/browse/JDK-8214799
>>> Webrev:
>>> http://cr.openjdk.java.net/~lkorinth/8214799/00/
>>> Testing:
>>> passed:
>>> mach5 remote-build-and-test --build-profiles 
>>> linux-x64,linux-x64-debug,macosx-x64,solaris-sparcv9,windows-x64 
>>> --test open/test/hotspot/jtreg/:hotspot_gc
>>> now running:
>>> mach5 remote-test --build-id 2018-12-05-1132377.leo.korinth.hs 
>>> --build-profiles linux-x64-debug -j tier1,tier2,tier3
>>> Thanks,
>>> Leo
>