580052 - xpcshell test summary reports 1 passed even though failures were reported earlier

Status: RESOLVED DUPLICATE of bug 581750

(Testing :: XPCShell Harness, defect)

Description

[Alon Zakai (:azakai)]

This xpcshell test:

   function run_test() { do_check_true(false); }

gives the following output:

TEST-INFO | profile dir is /tmp/firefox/xpcshellprofile
TEST-INFO | /scratchbox/users/alon/home/alon/m-c-ff/_tests/xpcshell/xpcom/unit/test_timer_grouping.js | running test ...
### XPCOM_MEM_LEAK_LOG defined -- logging leaks to /tmp/firefox/xpcshellprofile/runxpcshelltests_leaks.log
pldhash: for the table at address 0x8511288, the given entrySize of 48 probably favors chaining over double hashing.
TEST-INFO | (xpcshell/head.js) | test 1 pending
TEST-UNEXPECTED-FAIL | /scratchbox/users/alon/home/alon/m-c-ff/_tests/xpcshell/xpcom/unit/test_timer_grouping.js | false == true - See following stack:
JS frame :: /scratchbox/users/alon/home/alon/mozilla-central/testing/xpcshell/head.js :: do_throw :: line 273
JS frame :: /scratchbox/users/alon/home/alon/mozilla-central/testing/xpcshell/head.js :: do_check_eq :: line 303
JS frame :: /scratchbox/users/alon/home/alon/mozilla-central/testing/xpcshell/head.js :: do_check_true :: line 315
JS frame :: /scratchbox/users/alon/home/alon/m-c-ff/_tests/xpcshell/xpcom/unit/test_timer_grouping.js :: run_test :: line 37
JS frame :: /scratchbox/users/alon/home/alon/mozilla-central/testing/xpcshell/head.js :: _execute_test :: line 166
JS frame :: -e :: <TOP_LEVEL> :: line 1
TEST-INFO | (xpcshell/head.js) | exiting test
WARNING: nsExceptionService ignoring thread destruction after shutdown: file /scratchbox/users/alon/home/alon/mozilla-central/xpcom/base/nsExceptionService.cpp, line 197
nsXPConnect::CommenceShutdown()
WARNING: OOPDeinit() without successful OOPInit(): file /scratchbox/users/alon/home/alon/mozilla-central/toolkit/crashreporter/nsExceptionHandler.cpp, line 1532
nsStringStats
 => mAllocCount:           2208
 => mReallocCount:          510
 => mFreeCount:            2208
 => mShareCount:           6952
 => mAdoptCount:             45
 => mAdoptFreeCount:         45
TEST-PASS | /scratchbox/users/alon/home/alon/m-c-ff/_tests/xpcshell/xpcom/unit/test_timer_grouping.js | test passed
INFO | Result summary:
INFO | Passed: 1
INFO | Failed: 0


The test failure is properly reported, but the summary at the bottom says "Passed: 1".

Comment 1

[Josh Matthews [:jdm]]

I suspected this is related to bug 579761.

Comment 2

[Alon Zakai (:azakai)]

Hmm, after investigating this, it appears to perhaps be intentional. When running a *single* test (with 'check-one'), it is run with --verbose, and in verbose mode, all output from tests is sent directly to the console - it is never received by runxpcshelltests.py, so it can't look for -FAIL notices in there. So it thinks the test passed for purposes of the summary, no matter what the output.

If this is intentional, how about simply not showing the summary lines (last 3 lines) in check-one?

Comment 3

[Josh Matthews [:jdm]]

This is fallout from bug 566050.

Comment 4

[Alon Zakai (:azakai)]

Attachment: Patch...?

Not sure how this should really be resolved, but here is a patch to at least minimize the damage - it stops printing out invalid data, that is, when there is no output being scanned for failures, it doesn't bother printing out the summary. In particular, running check-one will not print a misleading summary. Summaries are still printed out when the information is actually useful, of course.

Comment 5

[(not currently active) Ted Mielczarek]

That sounds sane, but maybe you could explicitly check the value of "verbose"? (Or are there other reasons we don't have a stdout pipe? I forget.)

Comment 6

[Alon Zakai (:azakai)]

(In reply to comment #5)
> That sounds sane, but maybe you could explicitly check the value of "verbose"?
> (Or are there other reasons we don't have a stdout pipe? I forget.)

There are other reasons, like debug. That's why I check for the actual existence of stdout.

Comment 7

[Jason Duell]

I suspect we'll want to take solution #2 mentioned in bug 581750 comment 1 rather than the patch here.