[rfc][icedtea-web] known to fail annotation

[Jiri Vanek]

On 05/15/2012 05:10 PM, Jiri Vanek wrote:
> On 05/15/2012 04:53 PM, Omair Majid wrote:
>> Hi Jiri,
>>
>> On 05/15/2012 10:47 AM, Jiri Vanek wrote:
>>> On 05/15/2012 04:16 PM, Omair Majid wrote:
>>> Hi! Answered as expected ;)
>>
>> :)
>>
>>> Well. The reason why it should be included was recently discussed. A lot
>>> of tests are failing, because they are reproducing issue. Also I have
>>> plenty of new reproducers which should be definitely pushed. And if
>>> there will be to much failing reproducers then no one will see some issue.
>>>
>>> Now when somebody will break something by his changeset, and eg new
>>> failure will arise, it is easy to overlook new red line between
>>> reproducers. (both unit and "my")
>>
>> Ah. I see. So will things marked with this annotation not be run by
>> default? This sounds like a good idea to me.
>
> Yes!
>
> When you check my other KnownToFail needs-review, then you discover that in closest future I'm going
> just to mark them and print it out that it is expected.
>
> But yes - some by-configure enableing/disabling will definitely be worthy. What will be default is
> right now inappropriate but for sure they can be disabled/ignored. It is definietly the main reason
> of this mark.
>
>>
>>>>> I have dared also to extend it a bit for marking of tests which are not
>>>>> failing always, but are failing often by some abstract percentage of how
>>>>> often they are failing.
>>>>
>>>> I disagree with the concept of percentage. I don't see the point of
>>>> tests that (occasionally) fail. If tests fails all the time, that's
>>>> great. If tests don't fail all the time, the test is broken. I don't see
>>>> how the percentage helps in either case.
>>>
>>> Answer expected:) And I agree it it very discutable. The issue here is
>>> that I'm not able to reproduce the failure. And in case of cache
>>> reproducers (which are 90% of randomly failing reproducers) are actually
>>> representing the issue. Because cache is not protected from simultaneous
>>> read/write from multiple javaws. It is just luck when they sometimes
>>> pass (because of some delayed r/w on system)
>>>
>>
>> I am trying to figure out how the percentage helps. How do you plan to
>> use it?
>
> How to use it exactly can be seen in another KnownToFail patches. But I believe this is not what you
> are asking :)
>
> The reason in behind is the cache reproducers test-set:-/ It should be failing all the time, but
> sometimes i/o is to slow/fast and it pass.... Currently I have no idea how to make it better.
> Definitely it is worthy to fix it, but also it is quite on the bottom of todo list :((
> And for example the failure of classlaoder test I'm notable to reproduce on my local machine never :-/
>
> So this mark (that it is failing but not always) is some kind of warning. Currentlyu I'm just
> printing out that "this test is know to fail sometimes" (instead of "always"))
>
> How to deal with it when it will be disabled/enabled in future.. Well lI have some ideas - most easy
> looks like to have some border parameter. Which will be considered 100 or 0 by default.
>
>
>>
>> I still think the test is broken in this case. Perhaps we should fix the
>> test? Or consider rewriting/refactoring some code to make it more easily
>> testable? Or to put it another way, wouldn't effort spent adding this be
>> better spent fixing the code/tests?
>
> Yap - but sometimes the fix can be really hard. And until it is fixed, it is better to have it marked.
>
> But as I really like @knownToFail, I'm not 100% sure about this percentage too. Currently I'm about
> 65/35 for to include it.
>
> Thanx for your thought! J.

As agreed on IRC, Percentage removed. What do you think now?
After deep meditation  I consider it as better now too :)

J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: KnownToFailAnnotation_2-modifiedOutputOfTests.diff
Type: text/x-patch
Size: 20048 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120522/f9de6360/KnownToFailAnnotation_2-modifiedOutputOfTests.diff 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: KnownToFailAnnotation_2-apliedToTests-100.diff
Type: text/x-patch
Size: 4330 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120522/f9de6360/KnownToFailAnnotation_2-apliedToTests-100.diff 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: KnownToFailAnnotation_2-impl.diff
Type: text/x-patch
Size: 897 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120522/f9de6360/KnownToFailAnnotation_2-impl.diff