Closed
Bug 353010
Opened 18 years ago
Closed 17 years ago
test_all.sh's check for a test passing isn't stringent enough
Categories
(Testing :: XPCShell Harness, defect)
Testing
XPCShell Harness
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9
People
(Reporter: Waldo, Assigned: Waldo)
References
Details
Attachments
(3 files)
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.
Comment 1•18 years ago
|
||
Please attach a test case (heh) that should be reported as a failure, but is reported as PASS because of this issue.
Assignee | ||
Comment 2•18 years ago
|
||
Here's one that'll do the trick.
Assignee | ||
Comment 3•18 years ago
|
||
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
Assignee | ||
Comment 4•18 years ago
|
||
(The DIST="$bin" part is from a local change I have; ignore it, because it shouldn't make a difference in demonstrating failure.)
Comment 5•18 years ago
|
||
Jeff, if you have the log file handy, could you please attach that to this bug as well?
Assignee | ||
Comment 6•18 years ago
|
||
The output's all discombobulated because there's no output buffering, but you can imagine how it was supposed to work.
Assignee | ||
Comment 7•17 years ago
|
||
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 | ||
Updated•17 years ago
|
Attachment #288885 -
Flags: review? → review?(rcampbell)
Updated•17 years ago
|
Attachment #288885 -
Flags: review?(rcampbell) → review+
Assignee | ||
Comment 8•17 years ago
|
||
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.
Assignee | ||
Comment 9•17 years ago
|
||
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
Closed: 17 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M10
Updated•16 years ago
|
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.
Description
•