I can't find a bug already filed for this, just occasional mentions of it in bugmail, and we won't have a good idea of the scope of the problem or what needs to be fixed without having a bug. We hit about 1100 assertions when I ran the tests on Windows earlier today, with about 30-odd distinct assertions by source location. I'll file the necessary bugs and set the necessary dependencies when I have time, probably later today.

yeah, I just did a hunt through my bugmail and can't find anything specific on it, even though I know I've heard this requested before. Coop's currently working on getting leak-enabled debug builds running mochitest and reftest. We can use those machines to fail on assertion as well, if that makes sense.

Mass move of the rest of the Mochitest bugs from Core:Testing to Testing:Mochitest. Filter on MochitestMassMove to ignore.

dbaron has implemented the equivalent for reftest in bug 472557. We could use his new API to do this for Mochitest. We would need a way to annotate tests with the expected assertion count. Ideally we'd have a test manifest like reftest, but that's not quite simple. A simpler way might be to add a SimpleTest API that you can call like expectAssertions(5); (or something like that) to set the expected assertion count for your test, and having it default to zero, I guess.

The tricky part with mochitest will be dealing with assertions that happen while unloading the test. We might end up having to make the assertion checks work only when running the tests inside the harness and not happen while running a standalone test file. A potential bigger problem is tests that trigger assertions later, such as during GC; perhaps we can just assume that we won't have many of either type, and we'll try to fix all of that category before enabling the assertion checking. We've also yet to see how hard the reftest strategy will be to actually get working. We might end up with a good bit of pain with variable numbers of assertions. We're probably better off starting with some tests and getting some of the random/variable assertions fixed before moving on to all the tests.

We should be able to revisit this now that we have debug tinderboxes. I know dbaron has been cataloging the top assertions that we hit.

Currently we have (on Linux debug): 480 assertions in mochitest-plain-* (33 unique messages) 142 assertions in mochitest-chrome (12 unique messages) 485 assertions in mochitest-browser-chrome (19 unique messages) 6293 assertions in mochitest-a11y (15 unique messages) There are only 67 unique assertions that fire across all tests.

This seems to behave as expected (see patch description and in-code documentation). I'll give it a push-to-try to make sure.

Can we do this without using enablePrivilege? We're trying to remove that, and replace it with the specialpowers extension bits: http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/specialpowers/ I'd be happy to hack whatever you need to expose into SpecialPowers, I've been working on replacing our other enablePrivilege usage in the Mochitest harness.

It turns out there are various issues with a few tests, both with that patch, and with a different patch that avoided some of the issues (such as problems with some audio tests that call finish() multiple times). I ended up filing bug 631289 on one of the problems that happened with the second variant of the patch; we'll see what happens there, but I may just give up on trying to correctly attribute assertions that happen during unloading of the page. (After all, that still wouldn't handle assertions that happen during GC, etc.)

(In reply to comment #9) > Can we do this without using enablePrivilege? We're trying to remove that, and > replace it with the specialpowers extension bits: > http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/specialpowers/ > > I'd be happy to hack whatever you need to expose into SpecialPowers, I've been > working on replacing our other enablePrivilege usage in the Mochitest harness. All it needs is the isDebugBuild and assertionCount attributes from nsIDebug2.

I've had a work-in-progress patch for this for ages, currently: https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/fdf7f5b496e4/mochitest-assertion-checking (see log at https://hg.mozilla.org/users/dbaron_mozilla.com/patches/log/tip/mochitest-assertion-checking ) I just haven't had the chance to debug the failures that just the patch (never mind the assertion checking) causes yet; if my memory is correct they're currently in mochitest-a11y (though they used to be elsewhere).

And if I wait a few more months, again, the old failures go away and I get a new set of failures: https://tbpl.mozilla.org/?tree=Try&rev=969ff99bae00

You're not waiting until we don't suck, are you? Them's not failures, them's just the way we is.

I did rerun all the oranges to see if they were intermittent. Looks like some of them were (despite not matching the filed bugs).

(And, for the record, I did the same the previous times and the failures were not intermittent, and were rather more novel.)

OK, now that this seems to be green based on the failures going away over the last 10 months, I suppose it's time to try getting it in (before new failures are introduced). See description in commit message. I also started a more thorough (than the one in comment 13) try run: https://tbpl.mozilla.org/?tree=Try&rev=6482436439c9 (I'm not sure if the code to handle tests that call finish() more than once is actually still needed; I think the ones I knew about were fixed a while back.)

Comment on attachment 584885 [details] [diff] [review] assertion checking (initially not making tests fail) Review of attachment 584885 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/mochitest/tests/SimpleTest/TestRunner.js @@ +89,5 @@ > + netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect"); > + var DEBUG_CONTRACTID = "@mozilla.org/xpcom/debug;1"; > + var CC = Components.classes; > + var CI = Components.interfaces; > + var CR = Components.results; Nit: CR is unused, is it? @@ +372,5 @@ > + // chance to unload it. At some point in the future we should make > + // this an error. > + if (TestRunner._currentTest == TestRunner._lastTestFinished) { > + // > + TestRunner.logger.log("TEST-INFO | " + Nit: unable it and make it TEST-KNOWN-FAIL (or TEST-DETCEPXENU-FAIL?) ftb. @@ +414,5 @@ > + var iframe = $('testframe'); > + // For some reason, if we use an onload handler rather than an > + // inline script, then tests that call finish() multiple times cause > + // the onload to fail to fire in some cases (e.g., test_audio1 and > + // test_audio2, in succession, which both call finish three times). Nit: is that example still true? (Is there a bug filed?) @@ +445,5 @@ > + } > + if (numAsserts < min) { > + // WHEN ENABLING, change "log" to "error" and "DETCEPXENU" > + // to "UNEXPECTED". > + TestRunner.logger.log("TEST-DETCEPXENU-PASS | " + url + " | Assertion count " + numAsserts + " is less than expected range " + min + "-" + max + " assertions."); Logic should be: if (> max) else if (< min) else if (> 0)

(In reply to Serge Gautherie (:sgautherie) from comment #18) > > + // > > + TestRunner.logger.log("TEST-INFO | " + > > Nit: unable it and make it TEST-KNOWN-FAIL (or TEST-DETCEPXENU-FAIL?) ftb. s/unable/enable/ ;->

Comment on attachment 584885 [details] [diff] [review] assertion checking (initially not making tests fail) Review of attachment 584885 [details] [diff] [review]: ----------------------------------------------------------------- Overall this looks fine, but I don't want to add more enablePrivilege here. ::: testing/mochitest/tests/SimpleTest/TestRunner.js @@ +85,5 @@ > > TestRunner._expectingProcessCrash = false; > > +try { > + netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect"); Please add an API on SpecialPowers instead. We've been trying very hard to get rid of enablePrivilege in the harness, I'd rather not accumulate more uses of it.

Comment on attachment 610959 [details] [diff] [review] assertion checking (initially not making tests fail) Review of attachment 610959 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/mochitest/tests/SimpleTest/TestRunner.js @@ +397,5 @@ > + // Sometimes a test calls finish() multiple times before we have a > + // chance to unload it. At some point in the future we should make > + // this an error. > + if (TestRunner._currentTest == TestRunner._lastTestFinished) { > + TestRunner.error("TEST-UNEXPECTED-FAIL | " + Either you meant log(TEST-KNOWN-*) (or TEST-DETCEPXENU-*?), or you should update the previous comment too. @@ +447,5 @@ > + var iframe = $('testframe'); > + // For some reason, if we use an onload handler rather than an > + // inline script, then tests that call finish() multiple times cause > + // the onload to fail to fire in some cases (e.g., test_audio1 and > + // test_audio2, in succession, which both call finish three times). Nit: is that example still true? (Is there a bug filed?) @@ +469,5 @@ > + var min = TestRunner._expectedMinAsserts; > + if (numAsserts > max) { > + // WHEN ENABLING, change "log" to "error" and "DETCEPXENU" > + // to "UNEXPECTED". > + TestRunner.log("TEST-DETCEPXENU-FAIL | " + url + " | Assertion count " + numAsserts + " is greater than expected range " + min + "-" + max + " assertions."); Why not use TEST-KNOWN-* instead of TEST-DETCEPXENU-*? You could add a unique string to the description if need be. (everywhere) @@ +471,5 @@ > + // WHEN ENABLING, change "log" to "error" and "DETCEPXENU" > + // to "UNEXPECTED". > + TestRunner.log("TEST-DETCEPXENU-FAIL | " + url + " | Assertion count " + numAsserts + " is greater than expected range " + min + "-" + max + " assertions."); > + } else if (numAsserts > 0) { > + TestRunner.log("TEST-KNOWN-FAIL | " + url + " | Assertion count " + numAsserts + " within expected range " + min + "-" + max + " assertions."); Logic should be: if (> max) else if (< min) else if (> 0) Or that log is potentially wrong. @@ +473,5 @@ > + TestRunner.log("TEST-DETCEPXENU-FAIL | " + url + " | Assertion count " + numAsserts + " is greater than expected range " + min + "-" + max + " assertions."); > + } else if (numAsserts > 0) { > + TestRunner.log("TEST-KNOWN-FAIL | " + url + " | Assertion count " + numAsserts + " within expected range " + min + "-" + max + " assertions."); > + } > + // Separate from the if-else chain so we always fail when min > max. That erroneous case should be handled in expectAssertions(), not here.

(In reply to Serge Gautherie (:sgautherie) from comment #22) > Either you meant log(TEST-KNOWN-*) (or TEST-DETCEPXENU-*?), > or you should update the previous comment too. I've fixed the comment. > Nit: is that example still true? (Is there a bug filed?) I'll fix the comment. > Why not use TEST-KNOWN-* instead of TEST-DETCEPXENU-*? > You could add a unique string to the description if need be. > (everywhere) This technique works and I'd prefer to use it again. > Logic should be: > if (> max) > else if (< min) > else if (> 0) > > Or that log is potentially wrong. No, it shouldn't; see the comment: + // Separate from the if-else chain so we always fail when min > max. > That erroneous case should be handled in expectAssertions(), not here. I don't see why it matters.

(In reply to David Baron [:dbaron] from comment #23) > > Logic should be: > > if (> max) > > else if (< min) > > else if (> 0) > > > > Or that log is potentially wrong. > > No, it shouldn't; see the comment: Then you should fix the previous description because you have checked "0 < numAsserts <= max", not "min <= numAsserts <= max". > + // Separate from the if-else chain so we always fail when min > max. > > > That erroneous case should be handled in expectAssertions(), not here. > > I don't see why it matters. Because I always thought better to report/fix issues (like wrong data) asap rather than try to work around them later. Because your current code and descriptions don't match. Because, when "0 < numAsserts < min" or "max(!) < numAsserts < min", your current code logs two (contradictory) messages.

Comment on attachment 610959 [details] [diff] [review] assertion checking (initially not making tests fail) Review of attachment 610959 [details] [diff] [review]: ----------------------------------------------------------------- This looks fine to me, modulo what Serge has already pointed out.

I also needed some merging with https://hg.mozilla.org/mozilla-central/rev/bb0cc19274c5 to avoid having 2 definitions of isDebugBuild.

Turns out mochitest-chrome and mochitest-a11y no longer work (regressed sometime between late December and now): https://tbpl.mozilla.org/?tree=Try&rev=c4efe84be0f2 JavaScript error: data:text/html,<script>(parent.TestRunner%20||%20parent.wrappedJSObject.TestRunner).testUnloaded()</script>, line 1: Permission denied to access property 'TestRunner' Is anybody else interested in taking over trying to get this landed who could take over?

(A first step might be bisecting when it broke.) Note that the patch addressing review comments is here: https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/702d414a96e6/mochitest-assertion-checking and then with merging across changes from around the end of March (see comment 26) is here: https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/4c5281a74a5a/mochitest-assertion-checking

Still failing during mochitest-chrome and mochitest-a11y: https://tbpl.mozilla.org/?tree=Try&rev=752ab56b881e

David: looks like using postMessage works better; I sent a version with those changes to try at <https://tbpl.mozilla.org/?tree=Try&rev=fc8c671169f8>.

(Sorry, I didn't notice comment 30 until starting to write this comment, and the changes aren't accessible anymore.) In any case, I finally got around to moving the document to a separate file (I didn't think of trying postMessage), which seems to work, based on these try runs (the second having a fix for what broke the mochitest-2 and mochitest-ipcplugins in the first): https://tbpl.mozilla.org/?tree=Try&rev=f31e8bdcd84f https://tbpl.mozilla.org/?tree=Try&rev=13a3cca109b1 That said, maybe I should think about using postMessage. Ms2ger, do you still have that patch?

Oh, actually, try still has it; it's just the build results that are gone.

OK, new try run with Ms2ger's approach: https://tbpl.mozilla.org/?tree=Try&rev=359de52f597b

It turns out both approaches cause a reliable leak on Mac debug mochitest-2.

The leak is in the child process for test_browserElement_oop_SetVisibleFrames.html

I can reproduce the leak on Linux if I enable the test. The nsDocShell is held by two nsCOMPtrs from an HttpChannelChild/HttpBaseChannel. The channel is an nsHTMLDocument's mChannel. But if I take refcnt+comptr logs on the nsHTMLDocument (3 references leaked, 0 from nsCOMPtrs), the leak goes away. (Timing-sensitive?)

(In reply to David Baron [:dbaron] from comment #36) > But if I take refcnt+comptr logs on the nsHTMLDocument (3 references leaked, > 0 from nsCOMPtrs), the leak goes away. (Timing-sensitive?) And of the 3, the cycle collector knows about one, which is from the nodeinfomanager.

Oh, and the document leaked is the one for file_browserElement_SetVisibleFrames_Inner.html?child1

I filed bug 842476 on the leak.

So I tried some tweaks to make the leaving-a-page path more similar to the way it used to be. In particular: (1) using the _makeIframe function in TestRunner rather than setting iframe.src directly (2) reverting the comment 31 (etc.) changes to use postMessage rather than an interstitial page, so that we load the next page over http/chrome rather than data: Change (2) clearly fixes the problem in bug 842476 on Mac OS X 10.8 only; it remains on 10.7 and 10.6. It's possible that one or the other reduces the rate of mochitest-3 intermittent failures on Mac OS X (where I think the patch without both changes increased the rate of some existing intermittent failures), but it's hard to tell for sure. But I'm confused about the behavior of the mochitest-2 leak on Windows. Try runs for comparison: https://tbpl.mozilla.org/?tree=Try&rev=83062779b20a with neither change (older base) https://tbpl.mozilla.org/?tree=Try&rev=9c0247430fe7 with changes (1) and (2) https://tbpl.mozilla.org/?tree=Try&rev=788600749c21 with only change (1)

(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #40) > It's possible that one or the other reduces the rate of mochitest-3 > intermittent failures on Mac OS X (where I think the patch without both > changes increased the rate of some existing intermittent failures), but it's > hard to tell for sure. I think this was just random variation; in my earlier runs I just happened to hit some known intermittent failures.

https://tbpl.mozilla.org/?tree=Try&rev=31c9b4d81e6a is a try run with a workaround for bug 842476 (which makes the test not throw an exception, and thus not leak). I'm worried there's still a problem around dom/plugins/test/test_hanging.html

OK, I made one additional tweak, to call the parent's TestRunner from the load event of the interstitial document rather than directly from a <script></script> in it. That seems to be doing better so far: https://tbpl.mozilla.org/?tree=Try&rev=31587f8109d7 when combined with the workaround for bug 842476. (I think I tried it locally and it didn't fix bug 842476, so I think I'll still need that workaround)

https://hg.mozilla.org/integration/mozilla-inbound/rev/d1490171893b I realize I'm taking slight liberties with minor post-review modification here to get this green: * comment 31, which I'd like to replace with Ms2ger's postMessage approach * comment 40 item (1), which I'd like to keep, and * comment 43, which I probably would also like to keep (was noted as issue originally) Needs to stay open both for the first of those, for getting ted's confirmation that he's ok with those changes, and most importantly to actually move towards checking the assertions and reporting failures.

I looked over your changes as-landed and didn't see anything objectionable. I'm fine with you using some wiggle room to get this landed.

This script will let me download logs for analysis to build the table of current assertion counts. Liberally cribbed from https://hg.mozilla.org/users/dbaron_mozilla.com/buildbot-json-tools/

cribbed from bug 472557

cribbed from bug 472557

... and updated to actually work on this set of data

...needs dos2unix, now that tinderbox isn't in the pipeline

First (and hopefully primary) round of test annotations landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/4dc2f8fdfb55

If this is needed in further mochitests, I'd like to take your review+ as covering those as well.

If this is needed in further mochitests, I'd like to take your review+ as covering those as well.

Comment on attachment 718783 [details] [diff] [review] Do GC in a small number of tests to reduce the spread of GC-related assertions. Review of attachment 718783 [details] [diff] [review]: ----------------------------------------------------------------- Please file a bug on the assertion firing, and mention it in a comment (along with or instead of the assertion message). Does this change allow you to change expectAssertions(0, 1) to expectAssertions(1) in these tests, and remove it from some other tests? An unnecessarily lingering expectAssertions(0, 1) could hide other bugs.

Comment on attachment 718783 [details] [diff] [review] Do GC in a small number of tests to reduce the spread of GC-related assertions. Review of attachment 718783 [details] [diff] [review]: ----------------------------------------------------------------- It might be good to move the expectAssertions calls so they're next to the GC calls. So it's clear that they should both be removed once the assertion is fixed.

I'm going to do another patch on top to adjust the expectAssertions calls so they won't have a range anymore.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d08c12966908 https://hg.mozilla.org/integration/mozilla-inbound/rev/77ca1d9b5967 https://hg.mozilla.org/integration/mozilla-inbound/rev/710f17ddda96 https://hg.mozilla.org/integration/mozilla-inbound/rev/58890b4aadfb https://hg.mozilla.org/integration/mozilla-inbound/rev/4440a6338160 https://hg.mozilla.org/integration/mozilla-inbound/rev/672ce8b62c32

https://hg.mozilla.org/mozilla-central/rev/d08c12966908 https://hg.mozilla.org/mozilla-central/rev/77ca1d9b5967 https://hg.mozilla.org/mozilla-central/rev/710f17ddda96 https://hg.mozilla.org/mozilla-central/rev/58890b4aadfb https://hg.mozilla.org/mozilla-central/rev/4440a6338160 https://hg.mozilla.org/mozilla-central/rev/672ce8b62c32

https://hg.mozilla.org/integration/mozilla-inbound/rev/44625df8a140 https://hg.mozilla.org/integration/mozilla-inbound/rev/02b878360c64 (enabled!)

er, I meant https://hg.mozilla.org/integration/mozilla-inbound/rev/891d5589add6

The main course hasn't hit m-c yet.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c5cb505e1482 (In reply to Ryan VanderMeulen [:RyanVM] from comment #76) > https://hg.mozilla.org/integration/mozilla-inbound/rev/535ef5219dac I think I'd actually have chosen to file a bug there rather than annotate with a 0-inclusive range, but it's a close call.

Latest batch of failures: content/chrome/content/base/test/chrome/test_csp_bug768029.html | Assertion count 1 is greater than expected range 0-0 assertions. content/chrome/layout/generic/test/test_selection_preventDefault.html | Assertion count 2 is greater than expected range 0-1 assertions. content/chrome/layout/generic/test/test_selection_preventDefault.html | Assertion count 2 is greater than expected range 0-1 assertions. content/chrome/layout/generic/test/test_selection_preventDefault.html | Assertion count 3 is greater than expected range 0-2 assertions. content/chrome/layout/generic/test/test_selection_preventDefault.html | Assertion count 3 is greater than expected range 0-2 assertions. content/chrome/toolkit/content/tests/chrome/test_bug585946.xul | Assertion count 5 is greater than expected range 0-2 assertions. content/chrome/toolkit/content/tests/chrome/test_dialogfocus.xul | Assertion count 5 is greater than expected range 0-4 assertions. content/chrome/toolkit/content/tests/chrome/test_hiddenpaging.xul | Assertion count 2 is greater than expected range 0-0 assertions. content/chrome/toolkit/content/tests/chrome/test_keys.xul | Assertion count 2 is greater than expected range 0-0 assertions. content/chrome/toolkit/content/tests/chrome/test_keys.xul | Assertion count 2 is greater than expected range 0-0 assertions. content/chrome/toolkit/content/tests/chrome/test_keys.xul | Assertion count 2 is greater than expected range 0-0 assertions. content/chrome/toolkit/content/tests/chrome/test_keys.xul | Assertion count 3 is greater than expected range 0-0 assertions. content/chrome/toolkit/content/tests/chrome/test_largemenu.xul | Assertion count 2 is greater than expected range 0-0 assertions. dom/media/tests/mochitest/test_peerConnection_bug825703.html | Assertion count 11 is greater than expected range 0-0 assertions. dom/media/tests/mochitest/test_peerConnection_bug827843.html | Assertion count 184 is greater than expected range 0-0 assertions. dom/media/tests/mochitest/test_peerConnection_bug834153.html | Assertion count 214 is greater than expected range 0-0 assertions.

There were a few more than in the previous comment, annotated all of them in: https://hg.mozilla.org/integration/mozilla-inbound/rev/e8d3771bac8c dbaron, can I leave you to file the bugs?

Logs of things that need filing: https://tbpl.mozilla.org/php/getParsedLog.php?id=20170495&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=20172444&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=20172969&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=20172786&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=20177255&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=20175409&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=20176866&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=20175619&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=20178447&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=20177353&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=20176958&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=20178397&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=20179317&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=20178612&tree=Mozilla-Inbound

It's starting to look like we could have done with a few more try run retriggers to weed out the intermittent assertions, since there are yet more on inbound now :-( I've retriggered a number of the debug mochitest jobs on inbound to try and speed this process up.

https://hg.mozilla.org/mozilla-central/rev/44625df8a140 https://hg.mozilla.org/mozilla-central/rev/02b878360c64 https://hg.mozilla.org/mozilla-central/rev/56ee63cefc0b https://hg.mozilla.org/mozilla-central/rev/891d5589add6 https://hg.mozilla.org/mozilla-central/rev/43a54aaca03c https://hg.mozilla.org/mozilla-central/rev/934af7c3a7b0 https://hg.mozilla.org/mozilla-central/rev/b80de6ad9b08 https://hg.mozilla.org/mozilla-central/rev/2186eacb635c https://hg.mozilla.org/mozilla-central/rev/414a59f13ea4 https://hg.mozilla.org/mozilla-central/rev/2ba886324c76 https://hg.mozilla.org/mozilla-central/rev/97f66fd9a29a https://hg.mozilla.org/mozilla-central/rev/7b7e5220c420 https://hg.mozilla.org/mozilla-central/rev/535ef5219dac https://hg.mozilla.org/mozilla-central/rev/c5cb505e1482 https://hg.mozilla.org/mozilla-central/rev/2b18a04b0046 https://hg.mozilla.org/mozilla-central/rev/e8d3771bac8c

https://hg.mozilla.org/integration/mozilla-inbound/rev/65a2c1fd9d38 A bunch of these things are better off fixed in big batches (bug 846096, bug 846099, bug 845676, bug 683159, need bug on one more set that I'm forgetting) than individually annotated or filed.

(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #84) > A bunch of these things are better off fixed in big batches (bug 846096, bug > 846099, bug 845676, bug 683159, need bug on one more set that I'm > forgetting) than individually annotated or filed. Of the big sets of things that spew intermittent assertions across multiple tests: bug 846099 and bug 845676 and bug 683159 are now dealt with bug 671976 and bug 846096 still need to be dealt with

also still needs to be dealt with: bug 846137

also: bug 846150

https://hg.mozilla.org/mozilla-central/rev/65a2c1fd9d38 https://hg.mozilla.org/mozilla-central/rev/6f660b02c2a9

Current status of the big sets of intermittent things: dealt with: bug 846099, bug 845676, bug 683159, bug 671976 needs work: bug 846096 (needinfo? :smaug), bug 846137 (rare), bug 846150 (rare)

slow, but does what I needed

https://hg.mozilla.org/integration/mozilla-inbound/rev/d22db499ca12 It occurs to me that we should probably be noting the assertions we're annotating in the test.

https://hg.mozilla.org/mozilla-central/rev/ad3517d453c5 https://hg.mozilla.org/mozilla-central/rev/d22db499ca12

(In reply to Ryan VanderMeulen [:RyanVM] from comment #93) > It occurs to me that we should probably be noting the assertions we're > annotating in the test. Yes. In general we should be filing bugs on the assertions; it was just impossible to simultaneously do that for all the existing ones and get the patch landed. I filed followup bugs: bug 847275: make assertions cause test failures in mochitest-browser-chrome bug 847276: switch from an interstitial http: page back to a data: URL

Documentation for SimpleTest.expectAssertions would be nice.