(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #11)
(In reply to :Ehsan Akhgari from comment #9)
How are the test failures in the devtools test related to the test changes made in this patch? I see zero evidence of any such connection. In fact, it is impossible for there to be any such connection, since the patch that landed here makes changes to tests that run in a different suite, so there is zero chance of the changes here to have made any impact whatsoever to the test that fails intermittently here.
This is an issue how the backout was performed, maybe the first revision was still in the clipboad when then end of the range which shall be backed out got pasted.
Not sure what you mean, sorry? I'm not too familiar with the backout procedure so I can't decode the sentence.
You may ask yourself, could the failure in https://searchfox.org/mozilla-central/source/devtools/client/webconsole/test/mochitest/browser_webconsole_trackingprotection_errors.js have anything to do with bug 1525458 which also landed together with this patch? Looking at that test, the answer is no, since that test doesn't exercise the feature that this bug made changes to (url classifier notifications when the top-level page is on the content blocking allow list -- the test doesn't use the allow list at all.) So again no evidence for a connection there.
When bug 1525458 landed for the first time, it caused this test to fail: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&tochange=66ba790db6ff5210b1429173732a5816b39849d2&fromchange=d7d6b249119a5c8a8b179407f801152f09acb0b3&searchStr=devtools%2Clinux%2Copt&selectedJob=231055048 That's the first failure when this test started to fail frequently.
Of course it did, that first landing had a typo (https://bugzilla.mozilla.org/show_bug.cgi?id=1525458#c15) which completely disabled our tracking protection feature, so that test plus any other test in the tree which examined the TP feature would fail. But if you look at the failure message, it is completely different than what this was backed out for, since the second landing of bug 1525458 didn't have the typo. :-)
Looking at the log of the test, has this test been modified recently? Yes, it was modified just a couple of weeks ago in bug 1514824.
Frequent fails (except for edge cases) aren't triggered by a change weeks after it landed. It was correct to look for a recent change.
You are right, perhaps my theory isn't a great one. But that just suggests the culprit is something else based on the fact that a test change in one test suite cannot break a test in another test suite.
I certainly agree that looking for a recent change was the correct thing to do, no debate there!
Looking at all of the available evidence, it seems like what happened was the test had "tracking protection" in its name, and it had an intermittent failure bug in it, and instead of a bug being filed for it and the author of the test being asked to look at it, the latest patch that made a change to an unrelated test with "tracking" in its name got backed out. Based on this, I'm going to reland my patch and ask you to file a bug on this test failure and talk to Nicolas about looking at it. Thanks!
I will remind the sheriffs to talk to the developers if they believe to have found the change which causes a frequent failure.
Please be aware that the code sheriffs are not coding and thus are searching the culprits of failures by searching for test and folder names and doing backfills and retriggers.
Very fair, and thanks! This is indeed tricky to figure out, and I was only explaining the reason why I was certain the backout patch was innocent to demonstrate how I think about the issue when I saw it, in the hopes that some of that would be useful to sheriffs in the future when making other future tricky calls of this nature.
But of course this isn't a pure science so there will be misidentified regressors and that's part of the process. :-) I do appreciate the hard effort that the sheriffs spend in keeping our trees healthy. In a long long past I also used to spend some time sheriffing trees myself and I would get things wrong quite a few times in similar situations, so I definitely understand how hard this is from the other side, and I guess the last thing I'll say here is that our sheriffs are the force that keeps our development ship afloat on a day to day basis, so kudos for all of the hard work and thanks a lot for listening to the occasional complaints from folks like myself, please don't forget that we really appreciate all your hard work!