[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
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
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 o...@chromium.org 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?
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 e...@chromium.org 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] A new test in a patch passes locally fails on ews
I have converted the test into a text-only test. I didn't figure out the reason of the failure on bot. Today I encountered a similar issue with a ref test which fails on mac only. Why we stopped uploading zip files? I think it would be useful for debugging the issue and creating test expectations on ports that the developer doesn't have. (BTW, the link to the output (http://queues.webkit.org/results/14132702) in the message https://bugs.webkit.org/show_bug.cgi?id=98100#c19 seems incorrect. I can't find any occurrence of the name of the failing test in the linked page. The link from the ews icons under the patch is correct.) Xianzhu On Wed, Oct 3, 2012 at 12:13 AM, Adam Barth aba...@webkit.org wrote: The bots don't upload zip files any more. Your patch failed on both the chromium-ews and the mac-ews. You might try applying your patch to a clean working copy for one those ports to see if you can reproduce the failure. Adam On Tue, Oct 2, 2012 at 5:12 PM, Xianzhu Wang wangxian...@chromium.org wrote: Hi, I uploaded a patch containing a new test that passes locally, but failed on ews (https://bugs.webkit.org/show_bug.cgi?id=98100). I have no clue how it failed. The test doesn't seem to be flaky running locally. I remember in some cases the bot will give a zip file containing the results of the failed tests. Can I get it from ews? Thanks, Xianzhu ___ 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] A new test in a patch passes locally fails on ews
The zip files are sort of big, and we didn't want to spam bugs.webkit.org. Adam On Thu, Oct 4, 2012 at 4:28 PM, Xianzhu Wang wangxian...@chromium.org wrote: I have converted the test into a text-only test. I didn't figure out the reason of the failure on bot. Today I encountered a similar issue with a ref test which fails on mac only. Why we stopped uploading zip files? I think it would be useful for debugging the issue and creating test expectations on ports that the developer doesn't have. (BTW, the link to the output (http://queues.webkit.org/results/14132702) in the message https://bugs.webkit.org/show_bug.cgi?id=98100#c19 seems incorrect. I can't find any occurrence of the name of the failing test in the linked page. The link from the ews icons under the patch is correct.) Xianzhu On Wed, Oct 3, 2012 at 12:13 AM, Adam Barth aba...@webkit.org wrote: The bots don't upload zip files any more. Your patch failed on both the chromium-ews and the mac-ews. You might try applying your patch to a clean working copy for one those ports to see if you can reproduce the failure. Adam On Tue, Oct 2, 2012 at 5:12 PM, Xianzhu Wang wangxian...@chromium.org wrote: Hi, I uploaded a patch containing a new test that passes locally, but failed on ews (https://bugs.webkit.org/show_bug.cgi?id=98100). I have no clue how it failed. The test doesn't seem to be flaky running locally. I remember in some cases the bot will give a zip file containing the results of the failed tests. Can I get it from ews? Thanks, Xianzhu ___ 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] A new test in a patch passes locally fails on ews
What if we mail the zip files to the person that uploaded the patch? That way the responsibility of managing the storage is shifted to the author and the author still benefits from the results from all platforms. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
