Re: [webkit-dev] add NeedsRebaseline keyword to TestExpectations as a way to hande updating pixel tests?
I also like this idea. I particularly like the side effect of making it clear when a patch is modifying a test result that is itself not necessarily up to date. I suggest a policy that says you must rebaseline a test before committing a patch that would change that test (if the test is marked as needing rebaselining). Otherwise I see the potential for situations in which the status of a test gets confused due to overlapping patches or roll-outs. That also means we need an easy way to determine if a patch affects tests that are not reporting errors due to rebaselining. Can we have new-run-webkit-tests show, in the test results summary, the tests that you failed that need rebaselining? Cheers, Stephen. On Thu, Oct 4, 2012 at 6:18 PM, Emil A Eklund wrote: > That sounds like a great idea! > It is too hard to do this right today and having an easy way to > indicate that a test needs to be rebaselined across some or a subset > of platforms would be great. > > -- > Emil > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo/webkit-dev > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] add NeedsRebaseline keyword to TestExpectations as a way to hande updating pixel tests?
Makes sense to me ... On Thu, Oct 4, 2012 at 2:46 PM, Ojan Vafai wrote: > TL;DR: We should add a NeedsRebaseline keyword to TestExpectations and add > garden-o-matic tooling for it for the cases where someone commits a > change/test that they know will need new results for different ports (e.g. > any patch that changes the rendering of pixel tests). > > A common pattern that I see across ports is that someone will add something > like the following in a patch that changes the results of a pixel test: > // Needs rebaseline after r23456 > webkit.org/b/12345 path/to/test.html [ Failure ] > > This has a couple problems: > -Often the correct expectation is something like [ Missing Failure > ImageOnlyFailure ]. So, even though the test is listed, the bot turns red. > -The tooling can't give you a list of all the tests that are expected to > only need a rebaseline. > -Related to the above, people often forget about these lines and don't do > the rebaseline. > > We should add [ NeedsRebasline ], which is equivalent to [ Missing Failure > ImageOnlyFailure ]. I'm thinking it should not include Timeout/Crash since > those would need a solution other than a rebaseline (e.g. something is wrong > with the test or patch). > > In garden-o-matic, we can make a tab specifically for tests that need > rebaseline and give some indication whether the original patch that line was > added in has run on all the relevant bots. This way the people keeping the > tree green can also make sure that NeedsRebaseline lines don't get > forgotten. > > If it continues to be a problem we could even setup and automated nag bot to > email people who leave in NeedsRebaseline lines for more than a week. > > In the long-run, we should make it so you can grab the new results of the > EWS bots and don't need to add lines to TestExpectations at all. In the > short-term though, this is a way we can handle pixel tests without making > the tree red all the time. > > As a side note, we should also get rid of Missing as a valid expectation. A > test should either be NeedsRebaseline or have an expected result. > > Ojan > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo/webkit-dev > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] add NeedsRebaseline keyword to TestExpectations as a way to hande updating pixel tests?
That sounds like a great idea! It is too hard to do this right today and having an easy way to indicate that a test needs to be rebaselined across some or a subset of platforms would be great. -- Emil ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] add NeedsRebaseline keyword to TestExpectations as a way to hande updating pixel tests?
TL;DR: We should add a NeedsRebaseline keyword to TestExpectations and add garden-o-matic tooling for it for the cases where someone commits a change/test that they know will need new results for different ports (e.g. any patch that changes the rendering of pixel tests). A common pattern that I see across ports is that someone will add something like the following in a patch that changes the results of a pixel test: // Needs rebaseline after r23456 webkit.org/b/12345 path/to/test.html [ Failure ] This has a couple problems: -Often the correct expectation is something like [ Missing Failure ImageOnlyFailure ]. So, even though the test is listed, the bot turns red. -The tooling can't give you a list of all the tests that are expected to only need a rebaseline. -Related to the above, people often forget about these lines and don't do the rebaseline. We should add [ NeedsRebasline ], which is equivalent to [ Missing Failure ImageOnlyFailure ]. I'm thinking it should not include Timeout/Crash since those would need a solution other than a rebaseline (e.g. something is wrong with the test or patch). In garden-o-matic, we can make a tab specifically for tests that need rebaseline and give some indication whether the original patch that line was added in has run on all the relevant bots. This way the people keeping the tree green can also make sure that NeedsRebaseline lines don't get forgotten. If it continues to be a problem we could even setup and automated nag bot to email people who leave in NeedsRebaseline lines for more than a week. In the long-run, we should make it so you can grab the new results of the EWS bots and don't need to add lines to TestExpectations at all. In the short-term though, this is a way we can handle pixel tests without making the tree red all the time. As a side note, we should also get rid of Missing as a valid expectation. A test should either be NeedsRebaseline or have an expected result. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
