On Fri, Feb 21, 2014 at 4:55 AM, Gareth Aye <[email protected]> wrote: > Idea #2: Instead of Closing the Tree, Disable Tests and File Bugs > > Why do we close the tree when we find regressions? Because the broken tests > can no longer keep the corresponding features' functionality from > regressing even further.
We close the tree because the tree is broken and we don't know how to fix it. This means that we can't tell if further merges that happen contain more regressions. When tests are failing it means that there's a bug in the tree. Anyone that checks out the current tree risks having to deal with that bug. Either the bug is in the test, in which case the developer will have to figure out if the failure existed at the time when they pulled, or if the failure was something they introduced. Or worse, the bug is in the actual code, in which case the developer might be unable to work on their own bug/feature due to the current code not working. Fortunately, often the fix for failing tests is simple. Back out the patch that caused the regression! No need to close any trees. However it requires that we run tests when a merge happens and make it easy to see which test results correspond to which merge. And that someone is monitoring this and immediately back people out! The main case of this not working is intermittently failing tests. The best way to discover those is by keeping track of known intermittent issues and check against those when we see a failure that doesn't make sense (such as a merge for one app causing failures in another app). The only time when we need to close the tree is when tests are failing and we can't figure out what to back out in order to fix it. This includes both Gaia and gecko merges/checkins. > One idea is that, > whenever a regression pops up, we can simply disable the broken test and > file a high priority bug to diagnose the regression, revert an offending > patch or submit a fix, and re-enable the test. This does not work. In practice tests get disabled and no one works on re-enabling them. Also, disabling the test means that any further merges that happen while the test is disabled could be causing further breakages in the test. This makes the task of re-enabling a test even more annoying. Making it less likely that someone will bother to do it. And of course, disabling the test doesn't do anything to help the people pulling the tree and getting broken code. > Idea #3: Always Throttle the Tests to Proactively Discover Intermittent > Failures > > Why do we have regressions anyway? Who lands broken code in our tree? Can't > we scold them and get on with our lives? Well, it's not that simple for the > following two reasons. > > 1. Some tests only fail some of the time. This is a very real problem. We have a bunch of experience from Firefox desktop development here. But so far we've not been able to make the problem go away. Something that helps a lot is avoiding certain patterns in tests. Ehsan has written and done a lot of great work here. Something else that helps is better testing infrastructure that does things like automatically wait until the right element has been focused. I.e. utility functions available to test writers that makes it easier to write tests that do the right thing. And never use setTimeout!!! > 2. Projects which we're downstrseam from don't pay attention when they > break us. This should not happen. If we surface our tests on tbpl then they will be seen by everyone. That's mostly the case already, but we are missing critical tests like the integration tests. But we absolutely have to get our tests up on tbpl since that's the only place people look, and the only way gecko developers can try patches before committing them. There simply is no alternative. > I'll address the first issue here and the second one in the next section. > One idea that I've been championing on the mailing lists (and :evanxd has > recently introduced a patch to automate) is that we throttle tests on > checkins. > > Suppose Bob wrote the following contrived test using our most favorite test > harness <http://visionmedia.github.io/mocha/>: > > test('should work', function() { > assert.ok(Math.random() > 0.5); > }); > > Then suppose further that Bob submitted a patch with his test, saw his pull > request pass on CI, and merged his code. Then, all of a sudden, his test > started burning when Alice checked in a completely unrelated patch 10 > minutes later. *Oh noes!* If we had setup our CI to run Bob's test enough > times to tell with statistcal significance that it was passing, the whole > debacle could have been avoided! This would be good. Though we're already running into problems that we commit patches quicker than we can test them. Possibly if we could figure out subsets of tests that we could run for a given checkin we could run those tests multiple times. > Idea #4: Make our JS UI Tests Visible on TBPL Yes! This is a must! It's being actively worked on, and has been for quite a while by the A-team. > No matter how good our process and automation gets, we will have > regressions. We have a *very* complex project with lots of dependencies and > sometimes things just fall through the cracks. There are several tools that > might have helped us recover faster this week which either haven't been > built or haven't yet been used heavily in gaia. Some examples are: > > - Tools for gaia/gecko bisection (meaning to look into :jhford's project) This could be very cool if combined with the ability to run a specific test N times. This could help us find when an intermittently failing test started failing. > - Crash reporting for js ui tests Is this not already happening on tbpl? > - Tools uploading, sorting, and grouping screenshots taken via marionette Not sure I understand this one. Could you elaborate? / Jonas _______________________________________________ dev-b2g mailing list [email protected] https://lists.mozilla.org/listinfo/dev-b2g
