Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Wed, Mar 13, 2013 at 3:36 PM, Eric Seidel e...@webkit.org wrote: Given the history of that page, I'm not sure it truly reflects the consensus of the project: https://trac.webkit.org/wiki/CreatingLayoutTests?action=history Regardless of whether you are making a platform independent change or dependent change, it is your responsibility to ensure that the change does not negatively affect any other ports / platforms. This includes updating platform specific results or contacting a port maintainer to do this for you (for contacts, see: http://trac.webkit.org/wiki/WebKit%20Team). I'm not sure I see anyone following this these days. The EWS system and sherriffiing cultures seem to have largely replaced bot-monitoring. We also don't really have a system of core ports, thus all ports seems a bit broad here. Yeah, we probably need to loosen that rule up a little to reflect the current state of the world. It’s probably better to say something like “a patch author should be available on IRC and Bugzilla to answer questions bot maintainers have with respect to tests added or tests that have to be rebaselined”. Once your change is landed, there are some steps required to verify the change. Note: These steps apply for all changes. If you are making a platform dependent change, you should expect that additional results will be required for each platform but the actions are the same. I'm not sure it makes any sense to ask contributors to update other ports. In many cases, it's not possible for contributors to even build other ports. I think the point was that each contributor can grab results from build.webkit.org but I don’t think it’s really practical given the number of build bots we have today. We clearly need some sort of automated system for this, but right now I believe the expectation is that maintainers of the various ports will add the port-specific results, and that those contributing new layout tests should just land their results as the common ones? But I could be mistaken. That sounds like a reasonable resolution at least for now. I think the reality is that the project doesn’t really have consensus here, likely meaning this is a good topic for the contributors meeting. Yup. Meanwhile, it’s good to document what people do in practice so that new contributors don’t have to figure that out themselves. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
The steps described here differ from what I've been doing. In the interest of closure and port happiness, I've updated the wiki to reflect what is being proposed: https://trac.webkit.org/wiki/CreatingLayoutTests Please feel free to update it further. Philip On Tue, Feb 26, 2013 at 2:34 PM, Dirk Pranke dpra...@chromium.org wrote: On Tue, Feb 26, 2013 at 1:03 PM, Ryosuke Niwa rn...@webkit.org wrote: On Tue, Feb 26, 2013 at 12:47 PM, Dirk Pranke dpra...@chromium.org wrote: On Tue, Feb 26, 2013 at 2:11 AM, Ryosuke Niwa rn...@webkit.org wrote: On Tue, Feb 26, 2013 at 1:55 AM, Tom Hudson tomhud...@google.com wrote: On Mon, Feb 25, 2013 at 10:34 PM, Ryosuke Niwa rn...@webkit.org wrote: It should be fairly straight forward to create a tool that analyzes files changed in each commit and deduce which tests' expected results have been changed. The tool can then fetch results from each port' bot for those tests and automatically land them. It can then comment on the bug automatically about these rebaseline commits. There is no need to add remove entries from TestExpectation files. Wait, what? For some reason neither I nor the mailing list archives got your initial message, nor Silvia or Tom's responses, nor your responses (at least as of the time of me writing this), so I feel like I've missed a radical shift in this thread, and maybe I missed some of the context. https://lists.webkit.org/pipermail/webkit-dev/2013-February/023967.html This link doesn't point to any of those messages, but perhaps it's not that important. You're proposing that we automatically land updated baselines without review and then somehow update bugs, have people go back and look at the updated bugs to see if the baseline changes represent actual regressions or just expected changes? Right. Given that the commit already contains information as to which tests have been rebaselined, a script should be able to fetch new baselines for those affected tests on each platform and land them or upload as patches as needed. It's possible that we could fetch and cluster new baselines based on what changed in the initial commit. I would be concerned that there could be a fair amount of noise in either direction (tests that changed on the initial platform didn't on others, and others did), and you'd also have to figure out how to cluster changes since most builds on the bots contain multiple changes. But, you could probably use some of garden-o-matic's results to help here. That said, I'm not sure this workflow would actually improve things much over garden-o-matic. I am quite a bit more reluctant to automatically land any such changes; it seems like it would be hard if not impossible to tell (programmatically) whether a baseline changed as expected or if it represented a regression. If we were to work on new tooling, I would be much more in favor of pushing this up to an EWS-time step like Ossy suggests. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
Thanks for following up Philip. I don't feel like this thread met much consensus, more just went off into the weeds. Given the history of that page, I'm not sure it truly reflects the consensus of the project: https://trac.webkit.org/wiki/CreatingLayoutTests?action=history Regardless of whether you are making a platform independent change or dependent change, it is your responsibility to ensure that the change does not negatively affect any other ports / platforms. This includes updating platform specific results or contacting a port maintainer to do this for you (for contacts, see: http://trac.webkit.org/wiki/WebKit%20Team). I'm not sure I see anyone following this these days. The EWS system and sherriffiing cultures seem to have largely replaced bot-monitoring. We also don't really have a system of core ports, thus all ports seems a bit broad here. Once your change is landed, there are some steps required to verify the change. Note: These steps apply for all changes. If you are making a platform dependent change, you should expect that additional results will be required for each platform but the actions are the same. I'm not sure it makes any sense to ask contributors to update other ports. In many cases, it's not possible for contributors to even build other ports. We clearly need some sort of automated system for this, but right now I believe the expectation is that maintainers of the various ports will add the port-specific results, and that those contributing new layout tests should just land their results as the common ones? But I could be mistaken. I think the reality is that the project doesn't really have consensus here, likely meaning this is a good topic for the contributors meeting. On Wed, Mar 13, 2013 at 3:12 PM, Philip Rogers p...@google.com wrote: The steps described here differ from what I've been doing. In the interest of closure and port happiness, I've updated the wiki to reflect what is being proposed: https://trac.webkit.org/wiki/CreatingLayoutTests Please feel free to update it further. Philip On Tue, Feb 26, 2013 at 2:34 PM, Dirk Pranke dpra...@chromium.org wrote: On Tue, Feb 26, 2013 at 1:03 PM, Ryosuke Niwa rn...@webkit.org wrote: On Tue, Feb 26, 2013 at 12:47 PM, Dirk Pranke dpra...@chromium.org wrote: On Tue, Feb 26, 2013 at 2:11 AM, Ryosuke Niwa rn...@webkit.org wrote: On Tue, Feb 26, 2013 at 1:55 AM, Tom Hudson tomhud...@google.com wrote: On Mon, Feb 25, 2013 at 10:34 PM, Ryosuke Niwa rn...@webkit.org wrote: It should be fairly straight forward to create a tool that analyzes files changed in each commit and deduce which tests' expected results have been changed. The tool can then fetch results from each port' bot for those tests and automatically land them. It can then comment on the bug automatically about these rebaseline commits. There is no need to add remove entries from TestExpectation files. Wait, what? For some reason neither I nor the mailing list archives got your initial message, nor Silvia or Tom's responses, nor your responses (at least as of the time of me writing this), so I feel like I've missed a radical shift in this thread, and maybe I missed some of the context. https://lists.webkit.org/pipermail/webkit-dev/2013-February/023967.html This link doesn't point to any of those messages, but perhaps it's not that important. You're proposing that we automatically land updated baselines without review and then somehow update bugs, have people go back and look at the updated bugs to see if the baseline changes represent actual regressions or just expected changes? Right. Given that the commit already contains information as to which tests have been rebaselined, a script should be able to fetch new baselines for those affected tests on each platform and land them or upload as patches as needed. It's possible that we could fetch and cluster new baselines based on what changed in the initial commit. I would be concerned that there could be a fair amount of noise in either direction (tests that changed on the initial platform didn't on others, and others did), and you'd also have to figure out how to cluster changes since most builds on the bots contain multiple changes. But, you could probably use some of garden-o-matic's results to help here. That said, I'm not sure this workflow would actually improve things much over garden-o-matic. I am quite a bit more reluctant to automatically land any such changes; it seems like it would be hard if not impossible to tell (programmatically) whether a baseline changed as expected or if it represented a regression. If we were to work on new tooling, I would be much more in favor of pushing this up to an EWS-time step like Ossy suggests. -- Dirk ___ webkit-dev mailing list
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Tue, Feb 26, 2013 at 1:55 AM, Tom Hudson tomhud...@google.com wrote: On Mon, Feb 25, 2013 at 10:34 PM, Ryosuke Niwa rn...@webkit.org wrote: It should be fairly straight forward to create a tool that analyzes files changed in each commit and deduce which tests' expected results have been changed. The tool can then fetch results from each port' bot for those tests and automatically land them. It can then comment on the bug automatically about these rebaseline commits. There is no need to add remove entries from TestExpectation files. I think it's implied in other messages in the thread, but just to be explicit: this automated rebaseline commit feels like exactly the wrong thing to do. How can you rebaseline a test without SOME manual inspection? I know that mass layout rebaselines may not have every pixel checked, but there's tooling to help with that. Changes that are benign in one port may not be benign in other ports. Automatically landing changes to other ports' baselines risks corrupting our test data. I hate to repeat myself every time this conversation comes up but here it is: historically, only Chromium port used expected results as correct results and non-Chromium ports used expected results files to store the current state of the world including expected failures although this may have changed a little since non-Chromium ports started using TestExpectations instead of Skipped files. There is a significant danger in adding test expectations as opposed to committing expected failures because tests with Failure or ImageOnlyFailure expectation can regress further in any minute. I've encountered several severe regressions that would have caught if relevant tests had not been marked Failure or ImageOnlyFailure (in some cases needing rebaselines) for some minor rendering issues in the past. So no, it's not exactly the wrong thing to do. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Tue, Feb 26, 2013 at 12:47 PM, Dirk Pranke dpra...@chromium.org wrote: On Tue, Feb 26, 2013 at 2:11 AM, Ryosuke Niwa rn...@webkit.org wrote: On Tue, Feb 26, 2013 at 1:55 AM, Tom Hudson tomhud...@google.com wrote: On Mon, Feb 25, 2013 at 10:34 PM, Ryosuke Niwa rn...@webkit.org wrote: It should be fairly straight forward to create a tool that analyzes files changed in each commit and deduce which tests' expected results have been changed. The tool can then fetch results from each port' bot for those tests and automatically land them. It can then comment on the bug automatically about these rebaseline commits. There is no need to add remove entries from TestExpectation files. Wait, what? For some reason neither I nor the mailing list archives got your initial message, nor Silvia or Tom's responses, nor your responses (at least as of the time of me writing this), so I feel like I've missed a radical shift in this thread, and maybe I missed some of the context. https://lists.webkit.org/pipermail/webkit-dev/2013-February/023967.html You're proposing that we automatically land updated baselines without review and then somehow update bugs, have people go back and look at the updated bugs to see if the baseline changes represent actual regressions or just expected changes? Right. Given that the commit already contains information as to which tests have been rebaselined, a script should be able to fetch new baselines for those affected tests on each platform and land them or upload as patches as needed. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
Hi, Ryosuke Niwa írta: (snip) I don't like the idea that now I have to either use this tool or manually read through TestExpectations to find tests that need rebaseline. It's also annoying if we had all contributors add new lines to TestExpectations because those changes will almost always conflict when applying patches. In general, I don't like any solution that requires modifying TestExpectations files a lot. It's unneeded svn churn. It should be fairly straight forward to create a tool that analyzes files changed in each commit and deduce which tests' expected results have been changed. The tool can then fetch results from each port' bot for those tests and automatically land them. It can then comment on the bug automatically about these rebaseline commits. There is no need to add remove entries from TestExpectation files. I like your idea about the new tool with some little improvement. In my mind I see an improved EWS which analyzes the uploaded patch, determine which expected files are touched (assuming that the author updated the expected files at least on one platform). And then the EWS can run only these tests on its own platform and upload the result to bugzilla. It is much more cheaper than run all tests on all patches. That's why only Chromium and Apple Mac EWS run test ... And to tell the truth the waiting time is huge sometimes. This run tests on demand only can be a good compromise for ports don't have 8x8 cores EWS machine. I'm a little bit sceptic about automatical rebase commits. I prefer only uploading the new results to the bugzilla. And then the author and/or the port maintainer/gardener can review if the new results are correct or not. And then they can integrate the new baselines with the original patch (with a tool, of course) and land as one commit. (Or try one more round on EWS if the results are too old.) But unfortunately sometimes/regularly folks review and commit faster than style checker bot run. :) In this case we might need one more tool to check the bots automatically and help the gardeners work to be able rebase in one patch instead of separated patches for each port. br, Ossy ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Tue, Feb 26, 2013 at 1:03 PM, Ryosuke Niwa rn...@webkit.org wrote: On Tue, Feb 26, 2013 at 12:47 PM, Dirk Pranke dpra...@chromium.org wrote: On Tue, Feb 26, 2013 at 2:11 AM, Ryosuke Niwa rn...@webkit.org wrote: On Tue, Feb 26, 2013 at 1:55 AM, Tom Hudson tomhud...@google.com wrote: On Mon, Feb 25, 2013 at 10:34 PM, Ryosuke Niwa rn...@webkit.org wrote: It should be fairly straight forward to create a tool that analyzes files changed in each commit and deduce which tests' expected results have been changed. The tool can then fetch results from each port' bot for those tests and automatically land them. It can then comment on the bug automatically about these rebaseline commits. There is no need to add remove entries from TestExpectation files. Wait, what? For some reason neither I nor the mailing list archives got your initial message, nor Silvia or Tom's responses, nor your responses (at least as of the time of me writing this), so I feel like I've missed a radical shift in this thread, and maybe I missed some of the context. https://lists.webkit.org/pipermail/webkit-dev/2013-February/023967.html This link doesn't point to any of those messages, but perhaps it's not that important. You're proposing that we automatically land updated baselines without review and then somehow update bugs, have people go back and look at the updated bugs to see if the baseline changes represent actual regressions or just expected changes? Right. Given that the commit already contains information as to which tests have been rebaselined, a script should be able to fetch new baselines for those affected tests on each platform and land them or upload as patches as needed. It's possible that we could fetch and cluster new baselines based on what changed in the initial commit. I would be concerned that there could be a fair amount of noise in either direction (tests that changed on the initial platform didn't on others, and others did), and you'd also have to figure out how to cluster changes since most builds on the bots contain multiple changes. But, you could probably use some of garden-o-matic's results to help here. That said, I'm not sure this workflow would actually improve things much over garden-o-matic. I am quite a bit more reluctant to automatically land any such changes; it seems like it would be hard if not impossible to tell (programmatically) whether a baseline changed as expected or if it represented a regression. If we were to work on new tooling, I would be much more in favor of pushing this up to an EWS-time step like Ossy suggests. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] Best practices for landing new/changed layout test expectations?
I've noticed as of late several different approaches being used when adding/changing LayoutTests which need rebaselining on other platforms. Obviously we cannot expect developers to test/rebaseline on all platforms before landing given our current infrastructure. But what should we expect them to do? Currently some folks add failing expectations to other ports TestExpectations. Some add [skip]. Some even add them to the (new) global TestExpectations file. What's the proper course of action? I checked: http://trac.webkit.org/wiki/Keeping%20the%20Tree%20Green http://trac.webkit.org/wiki/Creating%20and%20Submitting%20Layout%20Tests%20and%20Patches http://trac.webkit.org/wiki/TestExpectations and didn't see the I expect this to fail/need rebaseline on other ports case discussed. I remember some discussion of a [rebaseline] keyword in TestExpectations, but I'm not sure that ever made it in? ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Mon, Feb 25, 2013 at 12:38 PM, Eric Seidel esei...@google.com wrote: I've noticed as of late several different approaches being used when adding/changing LayoutTests which need rebaselining on other platforms. Obviously we cannot expect developers to test/rebaseline on all platforms before landing given our current infrastructure. But what should we expect them to do? Currently some folks add failing expectations to other ports TestExpectations. Some add [skip]. Some even add them to the (new) global TestExpectations file. What's the proper course of action? I checked: http://trac.webkit.org/wiki/Keeping%20the%20Tree%20Green http://trac.webkit.org/wiki/Creating%20and%20Submitting%20Layout%20Tests%20and%20Patches http://trac.webkit.org/wiki/TestExpectations and didn't see the I expect this to fail/need rebaseline on other ports case discussed. Memory says that the last time this was raised [1], the consensus was to land the change, turn the tree red, notify as many gardeners / port maintainers as possible, and rebaseline as quickly as possible. I.e., don't add entries to TestExpectations. Of course, such a policy wouldn't play nicely w/ the EWS bots, and I think this discussion predated everyone switching over to TestExpectations (and certainly predated the generic expectations file). I don't find the above entirely satisfactory, but I also don't have any great alternatives to endorse given the existing tooling. Also, if the test fails generically (all ports), it probably shouldn't be landed. I'm not sure why you couldn't at least update the port you're developing on; if you don't, how do you know your fix is working at all? (Of course, there could be a case where a high-priority fix might break some low-priority tests). I remember some discussion of a [rebaseline] keyword in TestExpectations, but I'm not sure that ever made it in? The [ NeedsRebaseline ] enhancement is, as of yet, unimplemented and unclaimed [2]. It shouldn't be too hard for someone to try it if they're looking for a reason to explore the NRWT code :) -- Dirk [1] https://lists.webkit.org/pipermail/webkit-dev/2012-April/020250.html [2] https://bugs.webkit.org/show_bug.cgi?id=100415 ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Mon, Feb 25, 2013 at 12:52 PM, Dirk Pranke dpra...@google.com wrote: On Mon, Feb 25, 2013 at 12:38 PM, Eric Seidel esei...@google.com wrote: I've noticed as of late several different approaches being used when adding/changing LayoutTests which need rebaselining on other platforms. Obviously we cannot expect developers to test/rebaseline on all platforms before landing given our current infrastructure. But what should we expect them to do? Currently some folks add failing expectations to other ports TestExpectations. Some add [skip]. Some even add them to the (new) global TestExpectations file. What's the proper course of action? I checked: http://trac.webkit.org/wiki/Keeping%20the%20Tree%20Green http://trac.webkit.org/wiki/Creating%20and%20Submitting%20Layout%20Tests%20and%20Patches http://trac.webkit.org/wiki/TestExpectations and didn't see the I expect this to fail/need rebaseline on other ports case discussed. Memory says that the last time this was raised [1], the consensus was to land the change, turn the tree red, notify as many gardeners / port maintainers as possible, and rebaseline as quickly as possible. I.e., don't add entries to TestExpectations. Of course, such a policy wouldn't play nicely w/ the EWS bots, and I think this discussion predated everyone switching over to TestExpectations (and certainly predated the generic expectations file). I don't find the above entirely satisfactory, but I also don't have any great alternatives to endorse given the existing tooling. Also, if the test fails generically (all ports), it probably shouldn't be landed. I'm not sure why you couldn't at least update the port you're developing on; if you don't, how do you know your fix is working at all? (Of course, there could be a case where a high-priority fix might break some low-priority tests). Adding a new feature behind a build flag disabled everywhere by default. I remember some discussion of a [rebaseline] keyword in TestExpectations, but I'm not sure that ever made it in? The [ NeedsRebaseline ] enhancement is, as of yet, unimplemented and unclaimed [2]. It shouldn't be too hard for someone to try it if they're looking for a reason to explore the NRWT code :) I object to adding such a thing. People add and forget about these entries way too often: # Needs rebaseline fast/sub-pixel/inline-block-with-padding.html [ Failure ] # Rebaseline required after https://bugs.webkit.org/show_bug.cgi?id=101177 webkit.org/b/101177svg/dynamic-updates/SVGUseElement-dom-requiredFeatures.html [ ImageOnlyFailure Pass ] webkit.org/b/101177 svg/repaint/inner-svg-change-viewBox.svg [ ImageOnlyFailure Pass ] # These tests need to be rebaselined after https://bugs.webkit.org/show_bug.cgi?id=99984 webkit.org/b/99984 svg/as-image/image-preserveAspectRatio-all.svg [ ImageOnlyFailure Pass ] webkit.org/b/99984 svg/filters/feImage-preserveAspectRatio-all.svg [ ImageOnlyFailure Pass ] webkit.org/b/99984svg/dynamic-updates/SVGFEImageElement-dom-preserveAspectRatio-attr.html [ ImageOnlyFailure Pass ] webkit.org/b/99984svg/dynamic-updates/SVGFEImageElement-svgdom-preserveAspectRatio-prop.html [ ImageOnlyFailure Pass ] # Needs rebaseline after bug https://bugs.webkit.org/show_bug.cgi?id=104462 webkit.org/b/98687fast/css-generated-content/table-row-group-to-inline.html [ Failure ] webkit.org/b/104595 fast/css/empty-generated-content.html [ Failure ] # Needs a rebaseline after WK108429 lands. webkit.org/b/108429 svg/custom/text-ctm.svg [ Pass Failure ] # Transient. Needs rebaseline. webkit.org/b/103955 fast/repaint/caret-with-transformation.html [ Missing ] # Needs rebaseline. webkit.org/b/105574 editing/input/caret-at-the-edge-of-contenteditable.html [ Failure ] webkit.org/b/105574editing/input/reveal-caret-of-multiline-contenteditable.html [ Failure ] webkit.org/b/105574 editing/input/reveal-caret-of-multiline-input.html [ Failure ] webkit.org/b/105574fast/spatial-navigation/snav-div-overflow-scrol-hidden.html [ Failure ] # Rebaseline required after https://webkit.org/b/31397 webkit.org/b/107567 svg/batik/text/xmlSpace.svg [ Failure ] webkit.org/b/107567 fast/text/capitalize-empty-generated-string.html [ Failure ] webkit.org/b/107567 fast/text/whitespace/007.html [ Failure ] webkit.org/b/107567 fast/text/whitespace/006.html [ Failure ] webkit.org/b/107567 fast/inline/drawStyledEmptyInlinesWithWS.html [ Failure ] webkit.org/b/107567 tables/mozilla/bugs/bug1318.html [ Failure ] webkit.org/b/107567 fast/inline/drawStyledEmptyInlines.html [ Failure ] webkit.org/b/107567 tables/mozilla/bugs/bug113235-3.html [ Failure ] webkit.org/b/107567 css2.1/t0505-c16-descendant-01-e.html [ Failure ] webkit.org/b/107567 tables/mozilla/bugs/bug1188.html [ Failure ] webkit.org/b/107567 editing/selection/extend-by-sentence-001.html [ Failure ] webkit.org/b/107567 svg/carto.net/combobox.svg [ Failure ] # Needs rebaseline after https://bugs.webkit.org/show_bug.cgi?id=70634
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Mon, Feb 25, 2013 at 1:22 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 12:52 PM, Dirk Pranke dpra...@google.com wrote: On Mon, Feb 25, 2013 at 12:38 PM, Eric Seidel esei...@google.com wrote: I've noticed as of late several different approaches being used when adding/changing LayoutTests which need rebaselining on other platforms. Obviously we cannot expect developers to test/rebaseline on all platforms before landing given our current infrastructure. But what should we expect them to do? Currently some folks add failing expectations to other ports TestExpectations. Some add [skip]. Some even add them to the (new) global TestExpectations file. What's the proper course of action? I checked: http://trac.webkit.org/wiki/Keeping%20the%20Tree%20Green http://trac.webkit.org/wiki/Creating%20and%20Submitting%20Layout%20Tests%20and%20Patches http://trac.webkit.org/wiki/TestExpectations and didn't see the I expect this to fail/need rebaseline on other ports case discussed. Memory says that the last time this was raised [1], the consensus was to land the change, turn the tree red, notify as many gardeners / port maintainers as possible, and rebaseline as quickly as possible. I.e., don't add entries to TestExpectations. Of course, such a policy wouldn't play nicely w/ the EWS bots, and I think this discussion predated everyone switching over to TestExpectations (and certainly predated the generic expectations file). I don't find the above entirely satisfactory, but I also don't have any great alternatives to endorse given the existing tooling. Also, if the test fails generically (all ports), it probably shouldn't be landed. I'm not sure why you couldn't at least update the port you're developing on; if you don't, how do you know your fix is working at all? (Of course, there could be a case where a high-priority fix might break some low-priority tests). Adding a new feature behind a build flag disabled everywhere by default. I remember some discussion of a [rebaseline] keyword in TestExpectations, but I'm not sure that ever made it in? The [ NeedsRebaseline ] enhancement is, as of yet, unimplemented and unclaimed [2]. It shouldn't be too hard for someone to try it if they're looking for a reason to explore the NRWT code :) I object to adding such a thing. People add and forget about these entries way too often: The rationale for adding the keyword was precisely this ... it would make it easier to programmatically identify tests needing updated baselines and alert people :). Do you have an alternative you'd suggest for keeping people from forgetting (particularly in the case where we have an expected failure waiting on a bug fix)? -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Mon, Feb 25, 2013 at 1:29 PM, Dirk Pranke dpra...@google.com wrote: On Mon, Feb 25, 2013 at 1:22 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 12:52 PM, Dirk Pranke dpra...@google.com wrote: I remember some discussion of a [rebaseline] keyword in TestExpectations, but I'm not sure that ever made it in? The [ NeedsRebaseline ] enhancement is, as of yet, unimplemented and unclaimed [2]. It shouldn't be too hard for someone to try it if they're looking for a reason to explore the NRWT code :) I object to adding such a thing. People add and forget about these entries way too often: The rationale for adding the keyword was precisely this ... it would make it easier to programmatically identify tests needing updated baselines and alert people :). Do you have an alternative you'd suggest for keeping people from forgetting (particularly in the case where we have an expected failure waiting on a bug fix)? Just turn bots red. People maintaining bots will notice and rebaseline them as needed. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Mon, Feb 25, 2013 at 1:33 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 1:29 PM, Dirk Pranke dpra...@google.com wrote: On Mon, Feb 25, 2013 at 1:22 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 12:52 PM, Dirk Pranke dpra...@google.com wrote: I remember some discussion of a [rebaseline] keyword in TestExpectations, but I'm not sure that ever made it in? The [ NeedsRebaseline ] enhancement is, as of yet, unimplemented and unclaimed [2]. It shouldn't be too hard for someone to try it if they're looking for a reason to explore the NRWT code :) I object to adding such a thing. People add and forget about these entries way too often: The rationale for adding the keyword was precisely this ... it would make it easier to programmatically identify tests needing updated baselines and alert people :). Do you have an alternative you'd suggest for keeping people from forgetting (particularly in the case where we have an expected failure waiting on a bug fix)? Just turn bots red. People maintaining bots will notice and rebaseline them as needed. If there's a significant time lapse between failing test and bug fix, it seems like a bad idea to leave the tree red. Are you suggesting we should land a failling baseline in the meantime? -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Mon, Feb 25, 2013 at 1:39 PM, Dirk Pranke dpra...@google.com wrote: On Mon, Feb 25, 2013 at 1:33 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 1:29 PM, Dirk Pranke dpra...@google.com wrote: On Mon, Feb 25, 2013 at 1:22 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 12:52 PM, Dirk Pranke dpra...@google.com wrote: I remember some discussion of a [rebaseline] keyword in TestExpectations, but I'm not sure that ever made it in? The [ NeedsRebaseline ] enhancement is, as of yet, unimplemented and unclaimed [2]. It shouldn't be too hard for someone to try it if they're looking for a reason to explore the NRWT code :) I object to adding such a thing. People add and forget about these entries way too often: The rationale for adding the keyword was precisely this ... it would make it easier to programmatically identify tests needing updated baselines and alert people :). Do you have an alternative you'd suggest for keeping people from forgetting (particularly in the case where we have an expected failure waiting on a bug fix)? Just turn bots red. People maintaining bots will notice and rebaseline them as needed. If there's a significant time lapse between failing test and bug fix, it seems like a bad idea to leave the tree red. I don't think that's necessarily bad. If the port maintainer is not available on IRC/Bugzilla and can't assist you in rebaselining tests, I don't think you should be accountable for keeping bots green for them in practice. On the other hand, if all it takes to keep bots green is to commit new baselines, then why don't patch authors just do that? Other than people who are not yet committers, it doesn't make much sense for patch authors to be adding test expectations when they could simply commit new baselines after the fact unless you're landing a patch that requires a rebaseline of mass scale, in which case, adding test expectations temporarily might make sense. All too often, people add test expectations and forget about them because bots are green. It's the issue with incentives. People know they shouldn't leave bots red so they have a much higher inceptive to fix them on time. If they have added test expectations, then bots are green and other people won't be nagging them to fix it so they have much less inceptive to spend their time fixing bots. Are you suggesting we should land a failling baseline in the meantime? No. I'm suggesting patch authors perform their due diligence and either ask port maintainers to rebaseline or rebaseline tests themselves. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Mon, Feb 25, 2013 at 2:21 PM, Silvia Pfeiffer silvi...@chromium.orgwrote: On Tue, Feb 26, 2013 at 8:22 AM, Ryosuke Niwa rn...@webkit.org wrote: I object to adding such a thing. People add and forget about these entries way too often: # Needs rebaseline fast/sub-pixel/inline-block-with-padding.html [ Failure ] # Rebaseline required after https://bugs.webkit.org/show_bug.cgi?id=101177 webkit.org/b/101177svg/dynamic-updates/SVGUseElement-dom-requiredFeatures.html [ ImageOnlyFailure Pass ] webkit.org/b/101177 svg/repaint/inner-svg-change-viewBox.svg [ ImageOnlyFailure Pass ] # These tests need to be rebaselined after https://bugs.webkit.org/show_bug.cgi?id=99984 webkit.org/b/99984 svg/as-image/image-preserveAspectRatio-all.svg [ ImageOnlyFailure Pass ] webkit.org/b/99984 svg/filters/feImage-preserveAspectRatio-all.svg [ ImageOnlyFailure Pass ] webkit.org/b/99984svg/dynamic-updates/SVGFEImageElement-dom-preserveAspectRatio-attr.html [ ImageOnlyFailure Pass ] webkit.org/b/99984svg/dynamic-updates/SVGFEImageElement-svgdom-preserveAspectRatio-prop.html [ ImageOnlyFailure Pass ] # Needs rebaseline after bug https://bugs.webkit.org/show_bug.cgi?id=104462 webkit.org/b/98687fast/css-generated-content/table-row-group-to-inline.html [ Failure ] webkit.org/b/104595 fast/css/empty-generated-content.html [ Failure ] # Needs a rebaseline after WK108429 lands. webkit.org/b/108429 svg/custom/text-ctm.svg [ Pass Failure ] # Transient. Needs rebaseline. webkit.org/b/103955 fast/repaint/caret-with-transformation.html [ Missing ] # Needs rebaseline. webkit.org/b/105574editing/input/caret-at-the-edge-of-contenteditable.html [ Failure ] webkit.org/b/105574editing/input/reveal-caret-of-multiline-contenteditable.html [ Failure ] webkit.org/b/105574 editing/input/reveal-caret-of-multiline-input.html [ Failure ] webkit.org/b/105574fast/spatial-navigation/snav-div-overflow-scrol-hidden.html [ Failure ] # Rebaseline required after https://webkit.org/b/31397 webkit.org/b/107567 svg/batik/text/xmlSpace.svg [ Failure ] webkit.org/b/107567 fast/text/capitalize-empty-generated-string.html [ Failure ] webkit.org/b/107567 fast/text/whitespace/007.html [ Failure ] webkit.org/b/107567 fast/text/whitespace/006.html [ Failure ] webkit.org/b/107567 fast/inline/drawStyledEmptyInlinesWithWS.html [ Failure ] webkit.org/b/107567 tables/mozilla/bugs/bug1318.html [ Failure ] webkit.org/b/107567 fast/inline/drawStyledEmptyInlines.html [ Failure ] webkit.org/b/107567 tables/mozilla/bugs/bug113235-3.html [ Failure ] webkit.org/b/107567 css2.1/t0505-c16-descendant-01-e.html [ Failure ] webkit.org/b/107567 tables/mozilla/bugs/bug1188.html [ Failure ] webkit.org/b/107567 editing/selection/extend-by-sentence-001.html [ Failure ] webkit.org/b/107567 svg/carto.net/combobox.svg [ Failure ] # Needs rebaseline after https://bugs.webkit.org/show_bug.cgi?id=70634 webkit.org/b/109507compositing/contents-opaque/contents-opaque-background-color.html [ Failure ] webkit.org/b/109507compositing/contents-opaque/contents-opaque-layer-opacity.html [ Failure ] webkit.org/b/109507compositing/contents-opaque/contents-opaque-layer-transform.html [ Failure ] webkit.org/b/109507 compositing/rtl/rtl-fixed.html [ Failure ] webkit.org/b/109507 compositing/rtl/rtl-fixed-overflow.html [ Failure ] # Rebaseline required after https://webkit.org/b/95772 webkit.org/b/109209fast/text/international/bidi-ignored-for-first-child-inline.html [ Failure ] I think it's great to be able to create such a list! The bug at https://bugs.webkit.org/show_bug.cgi?id=100415 points to the idea of extending the garden-o-matic tool to create rebaselines based on the keyword. I like this idea - it would help everyone, including non-committers, deal with updating testexpectations and keep the tree green longer. I don't believe in making bots go read - that's a productivity stopper. I don't like the idea that now I have to either use this tool or manually read through TestExpectations to find tests that need rebaseline. It's also annoying if we had all contributors add new lines to TestExpectations because those changes will almost always conflict when applying patches. In general, I don't like any solution that requires modifying TestExpectations files a lot. It's unneeded svn churn. It should be fairly straight forward to create a tool that analyzes files changed in each commit and deduce which tests' expected results have been changed. The tool can then fetch results from each port' bot for those tests and automatically land them. It can then comment on the bug automatically about these rebaseline commits. There is no need to add remove entries from TestExpectation files. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Mon, Feb 25, 2013 at 2:36 PM, Rouslan Solomakhin rous...@chromium.orgwrote: On Mon, Feb 25, 2013 at 2:05 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 1:39 PM, Dirk Pranke dpra...@google.com wrote: Are you suggesting we should land a failling baseline in the meantime? No. I'm suggesting patch authors perform their due diligence and either ask port maintainers to rebaseline or rebaseline tests themselves. How should one go about finding the port maintainers? There're 200 people on http://trac.webkit.org/wiki/WebKit%20Team and 400 people on #webkit IRC. :-) That page clearly indicates who works on which port. We can also create a mechanism by which we find port maintainers who are currently online. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Mon, Feb 25, 2013 at 2:48 PM, Dirk Pranke dpra...@google.com wrote: On Mon, Feb 25, 2013 at 2:05 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 1:39 PM, Dirk Pranke dpra...@google.com wrote: Are you suggesting we should land a failling baseline in the meantime? No. I'm suggesting patch authors perform their due diligence and either ask port maintainers to rebaseline or rebaseline tests themselves. I think either you misunderstood my question, or I am misunderstanding your answer. I'm not asking who, I'm asking what ... If we know some tests are failing, and when we fix a bug the tests will start passing again (but that patch might not land for quite some time), what should we (anyone) do in the meantime? Leave the tree red, land incorrect -expected baselines so that we can catch changes in behavior, or add lines to TestExpectations? Many of the lines you cited fell into the last category. Either one of those two solutions would work (although I strictly advice we do the latter) when there are failing tests and we need a fix in order for those tests to pass but I'm not interested in discussing that matter at the moment. I'm specifically opposed to adding new entries to TestExpectations for the sole purpose of rebaselining them later. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Mon, Feb 25, 2013 at 3:53 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 2:48 PM, Dirk Pranke dpra...@google.com wrote: On Mon, Feb 25, 2013 at 2:05 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 1:39 PM, Dirk Pranke dpra...@google.com wrote: Are you suggesting we should land a failling baseline in the meantime? No. I'm suggesting patch authors perform their due diligence and either ask port maintainers to rebaseline or rebaseline tests themselves. I think either you misunderstood my question, or I am misunderstanding your answer. I'm not asking who, I'm asking what ... If we know some tests are failing, and when we fix a bug the tests will start passing again (but that patch might not land for quite some time), what should we (anyone) do in the meantime? Leave the tree red, land incorrect -expected baselines so that we can catch changes in behavior, or add lines to TestExpectations? Many of the lines you cited fell into the last category. Either one of those two solutions would work (although I strictly advice we do the latter) when there are failing tests and we need a fix in order for those tests to pass but I'm not interested in discussing that matter at the moment. I'm specifically opposed to adding new entries to TestExpectations for the sole purpose of rebaselining them later. One of the modes that the new generic TE file permits is to specify a generic Skip and overriding this with per-port Pass. This provides (IMO) a more manageable way to land a patch that you expect will require per-port mods to fully get the patch working on the primary ports. It also allows one to land a patch containing tests where a subsequent patch will provide the functionality to be tested. I recently used both of these modes to sub-divide a large patch and to incrementally verify (landing individual per-port fixes as needed) individual ports. Personally, I prefer this over the just turn bots red mode. I suspect the turn bots red approach results in greater churn, due to rollouts and re-landing, than the finer grain approach I outlined above. I agree it requires the committer to follow through on other ports and eventually remove the generic Skip and Pass rules (once it works everywhere), but we have to assume here (IMO) that committers will do the right thing and take responsibility for the process. [And if not, then remind them.] So I for one, would vote against the turn the bots red approach. Overall, I think that approach produces a higher interrupt load on our limited committer and reviewer resources. I think it would be useful to give other procedures an opportunity to be tried out so we can have more data for choosing between alternative approaches. Regards, Glenn ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Mon, Feb 25, 2013 at 3:36 PM, Glenn Adams gl...@skynav.com wrote: On Mon, Feb 25, 2013 at 3:53 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 2:48 PM, Dirk Pranke dpra...@google.com wrote: On Mon, Feb 25, 2013 at 2:05 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 1:39 PM, Dirk Pranke dpra...@google.com wrote: Are you suggesting we should land a failling baseline in the meantime? No. I'm suggesting patch authors perform their due diligence and either ask port maintainers to rebaseline or rebaseline tests themselves. I think either you misunderstood my question, or I am misunderstanding your answer. I'm not asking who, I'm asking what ... If we know some tests are failing, and when we fix a bug the tests will start passing again (but that patch might not land for quite some time), what should we (anyone) do in the meantime? Leave the tree red, land incorrect -expected baselines so that we can catch changes in behavior, or add lines to TestExpectations? Many of the lines you cited fell into the last category. Either one of those two solutions would work (although I strictly advice we do the latter) when there are failing tests and we need a fix in order for those tests to pass but I'm not interested in discussing that matter at the moment. I'm specifically opposed to adding new entries to TestExpectations for the sole purpose of rebaselining them later. One of the modes that the new generic TE file permits is to specify a generic Skip and overriding this with per-port Pass. This provides (IMO) a more manageable way to land a patch that you expect will require per-port mods to fully get the patch working on the primary ports. It also allows one to land a patch containing tests where a subsequent patch will provide the functionality to be tested. I recently used both of these modes to sub-divide a large patch and to incrementally verify (landing individual per-port fixes as needed) individual ports. Personally, I prefer this over the just turn bots red mode. I suspect the turn bots red approach results in greater churn, due to rollouts and re-landing, than the finer grain approach I outlined above. I agree it requires the committer to follow through on other ports and eventually remove the generic Skip and Pass rules (once it works everywhere), but we have to assume here (IMO) that committers will do the right thing and take responsibility for the process. [And if not, then remind them.] So I for one, would vote against the turn the bots red approach. Overall, I think that approach produces a higher interrupt load on our limited committer and reviewer resources. I think it would be useful to give other procedures an opportunity to be tried out so we can have more data for choosing between alternative approaches. This has been tried as many people including yourself are using this approach in landing and rebaselining patches. In particular, a lot of new contributors from Chromium port use this strategy. When I used to work for Google, I periodically went through all entries in TestExpectations files that needed rebaselines and manually rebaselined them myself because people just leave entries like I've cited in another email all the time. Quite frankly, I don't want it to be my (or anyone but patch author's) job to take care of all these stale entries people add. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Mon, Feb 25, 2013 at 5:27 PM, Ryosuke Niwa rn...@webkit.org wrote: Quite frankly, I don't want it to be my (or anyone but patch author's) job to take care of all these stale entries people add. I agree, but I think that sloppy follow-up doesn't mean the approach is necessarily bad. We have to police our own work and others all the time. I don't see that going away. In any case, it wouldn't be hard to add a command to webkitpy that reports extraneous/questionable entries in TE files. [Or perhaps there is already a tool that I haven't ran across.] ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Feb 25, 2013, at 4:27 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 3:36 PM, Glenn Adams gl...@skynav.com wrote: On Mon, Feb 25, 2013 at 3:53 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 2:48 PM, Dirk Pranke dpra...@google.com wrote: On Mon, Feb 25, 2013 at 2:05 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 1:39 PM, Dirk Pranke dpra...@google.com wrote: Are you suggesting we should land a failling baseline in the meantime? No. I'm suggesting patch authors perform their due diligence and either ask port maintainers to rebaseline or rebaseline tests themselves. I think either you misunderstood my question, or I am misunderstanding your answer. I'm not asking who, I'm asking what ... If we know some tests are failing, and when we fix a bug the tests will start passing again (but that patch might not land for quite some time), what should we (anyone) do in the meantime? Leave the tree red, land incorrect -expected baselines so that we can catch changes in behavior, or add lines to TestExpectations? Many of the lines you cited fell into the last category. Either one of those two solutions would work (although I strictly advice we do the latter) when there are failing tests and we need a fix in order for those tests to pass but I'm not interested in discussing that matter at the moment. I'm specifically opposed to adding new entries to TestExpectations for the sole purpose of rebaselining them later. One of the modes that the new generic TE file permits is to specify a generic Skip and overriding this with per-port Pass. This provides (IMO) a more manageable way to land a patch that you expect will require per-port mods to fully get the patch working on the primary ports. It also allows one to land a patch containing tests where a subsequent patch will provide the functionality to be tested. I recently used both of these modes to sub-divide a large patch and to incrementally verify (landing individual per-port fixes as needed) individual ports. Personally, I prefer this over the just turn bots red mode. I suspect the turn bots red approach results in greater churn, due to rollouts and re-landing, than the finer grain approach I outlined above. I agree it requires the committer to follow through on other ports and eventually remove the generic Skip and Pass rules (once it works everywhere), but we have to assume here (IMO) that committers will do the right thing and take responsibility for the process. [And if not, then remind them.] So I for one, would vote against the turn the bots red approach. Overall, I think that approach produces a higher interrupt load on our limited committer and reviewer resources. I think it would be useful to give other procedures an opportunity to be tried out so we can have more data for choosing between alternative approaches. This has been tried as many people including yourself are using this approach in landing and rebaselining patches. In particular, a lot of new contributors from Chromium port use this strategy. When I used to work for Google, I periodically went through all entries in TestExpectations files that needed rebaselines and manually rebaselined them myself because people just leave entries like I've cited in another email all the time. Quite frankly, I don't want it to be my (or anyone but patch author's) job to take care of all these stale entries people add. The problem of Chromium in the early days was that all tests were rebaselined when they failed. Too many tests turned the bots red at this time and the Chromium folks didn't (couldn't?) take attention of all these failing tests. We lost the chance to fix a lot of these bugs immediately during this time. It took months to cover these tests in bug reports and fix them finally. Some tests may still report that everything is ok, even if they are failing - untracked. I see the current needs rebaseline approach better, since the tests are at least logged. I do not have an opinion how they should be logged. Greetings, Dirk - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Mon, Feb 25, 2013 at 6:17 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 5:06 PM, Glenn Adams gl...@skynav.com wrote: On Mon, Feb 25, 2013 at 5:27 PM, Ryosuke Niwa rn...@webkit.org wrote: Quite frankly, I don't want it to be my (or anyone but patch author's) job to take care of all these stale entries people add. I agree, but I think that sloppy follow-up doesn't mean the approach is necessarily bad. We have to police our own work and others all the time. If we need to constantly remind people to do X, then X needs to be removed from the list of things we need to remember to do. Or, we need to automate it in the process, like having style bot serve as a gatekeeper for people that forget to run check-webkit-style on their own. I'm sure that most of us have forgotten to run it ourselves, but that doesn't mean we should remove the tool or the style bot. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Mon, Feb 25, 2013 at 5:38 PM, Glenn Adams gl...@skynav.com wrote: On Mon, Feb 25, 2013 at 6:17 PM, Ryosuke Niwa rn...@webkit.org wrote: On Mon, Feb 25, 2013 at 5:06 PM, Glenn Adams gl...@skynav.com wrote: On Mon, Feb 25, 2013 at 5:27 PM, Ryosuke Niwa rn...@webkit.org wrote: Quite frankly, I don't want it to be my (or anyone but patch author's) job to take care of all these stale entries people add. I agree, but I think that sloppy follow-up doesn't mean the approach is necessarily bad. We have to police our own work and others all the time. If we need to constantly remind people to do X, then X needs to be removed from the list of things we need to remember to do. Or, we need to automate it in the process, like having style bot serve as a gatekeeper for people that forget to run check-webkit-style on their own. I'm sure that most of us have forgotten to run it ourselves, but that doesn't mean we should remove the tool or the style bot. Right. Although I do run check-webkit-style by virtue of my running webkit-patch upload to upload patches. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Best practices for landing new/changed layout test expectations?
On Mon, Feb 25, 2013 at 6:28 PM, Silvia Pfeiffer silvi...@chromium.orgwrote: On Tue, Feb 26, 2013 at 9:34 AM, Ryosuke Niwa rn...@webkit.org wrote: It should be fairly straight forward to create a tool that analyzes files changed in each commit and deduce which tests' expected results have been changed. The tool can then fetch results from each port' bot for those tests and automatically land them. It can then comment on the bug automatically about these rebaseline commits. There is no need to add remove entries from TestExpectation files. The consequence of this is that it's possible that rebaselines get committed that may actually indicate a new bug. Since your suggestion is to attach this to the bug through which the patch landed, I think that's ok and the original author (or the reviewer) should feel obliged to follow up on the newly introduced bug by creating a new bug. If we can do this in a way that avoids the bots from going red, I think it's a good solution. I don't like a dogmatic approach like keeping bots green at all cost. We need to weigh costs of bots turning red versus asking every contributor to do extra things in order to keep bots green while noticing that some of our 400 contributors are bound to make mistakes every now and then. After all, those people who get affected by red bots are also people who submit patches and (hopefully) maintain bots. I'm also saddened by the fact that the first thing some people do when they see builds failing or some tests failing is to blame others or complain instead of trying to fix those problems themselves. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev