test_all.sh checks for the success of a test by grepping for "*** PASS" in the log file. This is insufficient to check for test success, because it's entirely possible for a test to run and indicate success but for something to happen after this which causes an assertion to occur (which is intended to be fatal for tests). The simplest such case of this is one where an asynchronous test is improperly written such that the test indicates success *before* all callbacks from XPCOM objects occur; this usually causes the "no Components" assertion, but since that assertion occurs after "*** PASS" has been written to the file, it isn't caught by test_all.sh as an error. Probably we just need to check the return value of the line that invokes xpcshell, but I don't know enough to say for certain that that's all that needs to happen.
Please attach a test case (heh) that should be reported as a failure, but is reported as PASS because of this issue.
Created attachment 238857 [details] Testcase causes assertion, doesn't cause tests to fail Here's one that'll do the trick.
make check output for this is: ../../../dist/bin/run-mozilla.sh ../../../dist/bin/test_all.sh ../../../dist/bin/necko_unit_tests ../../../dist/bin/necko_unit_tests/test_assertion.js: ../../../dist/bin/test_all.sh: line 83: 20722 Aborted (core dumped) DIST="$bin" $bin/xpcshell $headfiles -f $t $tailfiles 2>$t.log 1>&2 PASS
(The DIST="$bin" part is from a local change I have; ignore it, because it shouldn't make a difference in demonstrating failure.)
Jeff, if you have the log file handy, could you please attach that to this bug as well?
Created attachment 238860 [details] Log file The output's all discombobulated because there's no output buffering, but you can imagine how it was supposed to work.
Created attachment 288885 [details] [diff] [review] Patch This fixes the problem for me, as demonstrated by the current version of toolkit/mozapps/extensions/test/test_bug378216.js (passes before this patch, correctly fails after -- the assertion is bug 403180). We need that bug to be fixed before this can be committed, because otherwise we'll break |make check| for anyone running it in a debug build.
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
Attachment #288885 - Flags: review?
Attachment #288885 - Flags: review? → review?(rcampbell)
Attachment #288885 - Flags: review?(rcampbell) → review+
Comment on attachment 288885 [details] [diff] [review] Patch Since this is a test-only change it doesn't need approval; I'll commit this after the fix for bug 403180 is reviewed and lands.
Checked in, along with tweaks to the XPCOM_DEBUG_BREAK variables in the scripts for stack-and-abort instead of just abort while I was there.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M10
Component: Testing → TUnit
Product: Core → Testing
QA Contact: testing → tunit
Target Milestone: mozilla1.9beta2 → mozilla1.9
You need to log in before you can comment on or make changes to this bug.