RFR: 8051984: @ignore should be placed after @test

[Andrey Zakharov]

16.02.2015 18:41, Bengt Rutisson пишет:
>
> Hi Andrey,
>
> On 16/02/15 16:24, Andrey Zakharov wrote:
>> Hi
>>> But JDK-8019361 looks like a wrong number.  It must be JDK-8051984, I think.
>>
>> I'm wrongly got reason of ignore bug for RFR. Thanks, Dima.
>>
>> webrev:
>> http://cr.openjdk.java.net/~azakharov/8051984/webrev//
>>
>> /bug:
>> https://bugs.openjdk.java.net/browse/JDK-8051984
>>
>>> If so, what about other sources listed in the description:
>>>     ./test/gc/arguments/TestParallelHeapSizeFlags.java
>>>     ./test/gc/arguments/TestUseCompressedOopsErgo.java
>>>     ./test/gc/g1/TestHumongousShrinkHeap.java
>> Its already fixed either by removing @ignore either by inserting 
>> @requires
>> /
>> /
>>> Why did you change the static import of
>>> com.oracle.java.testlibrary.Asserts? Seems unrelated to the @ignore
>>> change and I don't think there is a reason for it either. We use static
>>> import of the asserts a lot in our test code.
>>>
>>> Thanks,
>>> Bengt
>>>
>>> >/
>>> />/  Thanks./
>>
>> There is nothing especial in import static here, its  only serves to 
>> reduce Asserts package names in code, but it also leads to less 
>> readability and question like "what assertLessThan comes from?".
>> Asserts.assertLessThan is better - it doesn't junk global namespace. 
>> In only this case - its only code style question. If you have any 
>> other conserns about this, please tell me.
>
> For people reading the hotspots tests a lot I think assertLessThan() 
> is more readable than Asserts.assertLessThan(). Please don't change 
> this for this test only. If you feel strongly about this issue I think 
> you should bring it up for a wider discussion so we can agree on a 
> guideline for how to use it. Right now it just seems like a completely 
> unrelated change that is not necessary to solve your bug.
Ok. At least I must change Asserts.* to Asserts.assertLessThan;

webrev:
http://cr.openjdk.java.net/~azakharov/8051984/webrev.02/

Just for information:
in src/hs-gc/hotspot/test

grep 'import com.oracle.java.testlibrary.Asserts.' -R ./ 
--exclude-dir=.hg | wc -l
56

grep 'import static com.oracle.java.testlibrary.Asserts.' -R ./ 
--exclude-dir=.hg | wc -l
66

grep 'import static com.oracle.java.testlibrary.Asserts.' -R ./gc 
--exclude-dir=.hg | wc -l
17

grep 'import com.oracle.java.testlibrary.Asserts.' -R ./gc 
--exclude-dir=.hg | wc -l
5

Thanks.

>
> Thanks,
> Bengt
>
>>
>>
>> Thanks.
>>
>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150216/88db0a10/attachment.htm>