RFR(S) : 8078450 : Implement consistent process for quarantine of tests
dmitry.fazunenko at oracle.com
Tue Dec 6 12:29:22 UTC 2016
New version looks good to me.
See a few comments inlined
On 06.12.2016 15:07, Igor Ignatyev wrote:
> thank you for your review and comments, please see my answers inline.
> http://cr.openjdk.java.net/~iignatyev///8078450/webrev.01 is the updated webrev.
> — Igor
>> On Dec 5, 2016, at 4:35 PM, Dmitry Fazunenenko <dmitry.fazunenko at oracle.com> wrote:
>> Hi Igor,
>> Thank you for doing that long-awaited change!
>> The fix looks good, but I have a few minor questions/comments:
>> 1) Have you moved all the tests which are currently @ignored to the problem list or some of them intentionally left @ignored?
> I've moved all tests, at least all which were in the repo when I created a patch. to me, all of them seem quarantined tests, not excluded.
> have I missed some? or do you think some of them should be excluded?
Good. It means no 'excluded' tests for the time being.
>> 2) Copyrights
>> 3) Would you consider correction in wording:
>> # List of quarantined tests -- tests that should not be run by default, because
>> # they introduce noise in test results.
>> # List of problematic tests -- tests that should not be run by default, because
>> # they may fail due to known reason. The reason (CR#) must be mandatory specified.
> I’d stick w/ quarantined, since we have been using "quarantined” in that meaning for a while, and besides “problematic” is too generic, it’s also a new work, and I’d prefer not to pollute our dictionary w/ another type of failing tests.
Fine by me.
>> 4) Would you consider to use the same formatting as jdk does:
> I’d rather not, having such alignment doesn’t add much value, but costs a lot, because almost each change would modify number of lines around.
No objection, formatting is just a matter of taste.
>> On 02.12.2016 22:27, Igor Ignatyev wrote:
>>>> 115 lines changed: 97 ins; 17 del; 1 mod;
>>> Hi all,
>>> could you please review this small changeset which introduces ProblemList for quarantining hotspot test and adds all currently quarantined tests to it?
>>> a bit of background:
>>> there are two cases when one wants to remove a test from test execution:
>>> - a test sporadically fails, but there is still a value in running this test, e.g. it can fail in some other way and reveal another problem within the product. we want to run such tests, but we don’t want to have noise from these tests. to get both, we are going to _quarantine_ these tests and run them separately.
>>> - a test always fails or can break other tests or host. running such tests makes more harm than good, so they should be _excluded_ from test execution completely.
>>> jtreg provides two exclusion mechanisms: @ignore tag and ProblemList. ProblemList gives a possibility to remove a test from execution on a specific OS, arch, and since the expectation is to have more quarantined tests than excluded, it was decided to use ProblemList to quarantine and @ignore to exclude, another reason is to have consistent process for hotspot and jdk tests.
>>> in two words, @ignore to exclude, ProblemList to quarantine.
>>> webrev: http://cr.openjdk.java.net/~iignatyev/8078450/webrev.00/
>>> jbs: https://bugs.openjdk.java.net/browse/JDK-8078450
>>> — Igor
More information about the hotspot-dev