Closed Bug 733501 Opened 9 years ago Closed 8 years ago
crashreporter tests seem to be skipped everywhere
These tests are supposed to run as part of X jobs, but I can't find any jobs where they're actually running. https://tbpl.mozilla.org/php/getParsedLog.php?id=9846746&tree=Firefox TEST-INFO | skipping /Users/cltbld/talos-slave/test/build/xpcshell/tests/toolkit/crashreporter/test/unit/test_crash_oom.js | skip-if: os == "linux" || !crashreporter
These are only supposed to be skipped on Linux or builds with --disable-crashreporter: http://mxr.mozilla.org/mozilla-central/source/testing/xpcshell/xpcshell.ini#85 So it's possible that we either broke the xpcshell harness or our use of MozInfo or something.
I found the root cause here while working on bug 746546. The patch there should fix this, I've pushed it to try to see if it breaks anything else. It turns out that when we call the writemozinfo script, which produces the JSON that is used to key the expressions in the manifest: http://mxr.mozilla.org/mozilla-central/source/configure.in#8964 we're neglecting to propogate the value of MOZ_CRASHREPORTER to it, so it fails to set crashreporter==true. :-(
Assignee: nobody → ted.mielczarek
Depends on: 746546
Of course some of these tests fail in exciting ways: https://tbpl.mozilla.org/?tree=Try&rev=7854b4ae2add (ignore the OS X opt build failure, that's from something else in my patch). test_crashreporter_crash.js is failing on WinXP, but that's just because it's calling do_check_true on the result of String.match, which returns a string (and do_check_true just literally checks == true). test_crash_oom.js is hanging on Win7. I don't know what's going on there.
This patch fixes the stupid mistake in configure, and also a few other fiddly things. We had these tests disabled on Linux, so I re-enabled them in the xpcshell.ini manifest. They all pass on my machine, but I'll push to try and retrigger a bunch of jobs to make sure they're not flaky there. Some of the tests were also broken by some XPConnect change (filed as bug 781628), so I rolled that fix into this patch.
Attachment #661805 - Flags: review?(jmaher)
Also I've only tested this patch on top of my "Update Breakpad to latest SVN" patch, so I'm going to land them together.
Comment on attachment 661805 [details] [diff] [review] Fix crashreporter xpcshell tests to actually run Review of attachment 661805 [details] [diff] [review]: ----------------------------------------------------------------- these changes look pretty good. Make sure they run on try server first.
Attachment #661805 - Flags: review?(jmaher) → review+
Interestingly, test_crash_oom.js fails on Windows debug builds because the debug allocator asserts when we try to allocate that much memory. On my try builds this shows up as a hang because it's displaying the CRT assert dialog. I guess we probably ought to use the code from bug 587982 in xpcshell as well (possibly by factoring it out into a common place). I think I might just skip this test on windows debug builds. I suppose this means we're not using jemalloc on Windows debug, though.
Updated to skip test_crash_oom.js on Windows debug builds for now. This is the patch I'm going to land.
Attachment #661805 - Attachment is obsolete: true
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/c1e97e2838a7 along with bug 791775
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.