Closed Bug 967564 Opened 6 years ago Closed 6 years ago

Assert.jsm should report operands when test fails

Categories

(Testing :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla30

People

(Reporter: gps, Assigned: mikedeboer)

Details

Attachments

(1 file)

I'm not sure if this is a Assert.jsm core bug or a bug with the xpcshell integration with Assert.jsm. Anyway, error messages from Assert.equal and other test functions don't seem to report the values of operands when they fail. This makes deciphering failures without code changes to add additional debug output difficult.

  Assert.equal(id, "bar", "ID recorded properly");

Will yield the following unhelpful error:

 0:02.74 TEST-UNEXPECTED-FAIL | /Users/gps/src/firefox/obj-firefox.noindex/_tests/xpcshell/toolkit/crashreporter/test/unit/test_event_files.js | ID recorded properly - See following stack:
 0:02.74 JS frame :: /Users/gps/src/firefox/obj-firefox.noindex/_tests/xpcshell/toolkit/crashreporter/test/unit/test_event_files.js :: test_main_process_crash :: line 41
 0:02.74 JS frame :: resource://gre/modules/Task.jsm :: TaskImpl_run :: line 217
 0:02.74 JS frame :: resource://gre/modules/commonjs/sdk/core/promise.js :: resolve :: line 118
 0:02.74 JS frame :: resource://gre/modules/commonjs/sdk/core/promise.js :: then :: line 43
 0:02.74 JS frame :: resource://gre/modules/commonjs/sdk/core/promise.js :: then :: line 153
 0:02.74 JS frame :: resource://gre/modules/Task.jsm :: TaskImpl_handleResultValue :: line 269
 0:02.74 JS frame :: resource://gre/modules/Task.jsm :: TaskImpl_run :: line 225
 0:02.74 JS frame :: resource://gre/modules/Promise.jsm :: Handler.prototype.process :: line 767
 0:02.74 JS frame :: resource://gre/modules/Promise.jsm :: this.PromiseWalker.walkerLoop :: line 531
 0:02.74 native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
 0:02.74 TEST-INFO | (xpcshell/head.js) | exiting test
@Gregory, yeah that's an 'intentional' side-effect of providing your own message: http://dxr.mozilla.org/mozilla-central/source/testing/modules/Assert.jsm#86

I could make it so it'd prefix the message returned from `getMessage()` with the optionally provided message + " - ". What do you think?
Flags: needinfo?(gps)
On failures, I think we should provide enough context to diagnose the problem.

Think of the TBPL use case. You see an intermittent in automation. Your first question is "what values caused the comparison to fail." We need this info in the output, even if a custom message is used. Custom messages serve to provide additional context to help debugging. They shouldn't be a replacement for actual values.
Flags: needinfo?(gps)
Agreed.
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Comment on attachment 8370146 [details] [diff] [review]
Patch v1: report operand, even when a custom message is specified

Review of attachment 8370146 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM!

Thanks for the rapid turnaround.
Attachment #8370146 - Flags: review?(gps) → review+
Could've pushed earlier, but ok :)

remote: https://hg.mozilla.org/integration/fx-team/rev/62dd35d50578
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/62dd35d50578
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.