Add line numbers to assertions

RESOLVED FIXED in Firefox 42

Status

Testing
Mochitest
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: Martijn Wargers (zombie), Assigned: hiro)

Tracking

unspecified
mozilla42
Points:
---

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

4 years ago
Created attachment 8524708 [details] [diff] [review]
poc patch

From bug 744387, comment 5:
"
maybe we can add a line number into the output to make assertions clearer? (I've wanted that even for assertions with a message, it's not always clear which one they are.)
"

The poc patch does something like that.
(Assignee)

Updated

3 years ago
Duplicate of this bug: 980098
(Assignee)

Comment 2

3 years ago
Created attachment 8598457 [details] [diff] [review]
A simple solution

This patch outputs stack traces on console if assertion fails like this:

3446 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/general/test_resource_timing.html | Performance.getEntries() returned wrong number of entries. - got 4, expected 5
SimpleTest.is@tests/SimpleTest/SimpleTest.js:286:5
is@tests/dom/tests/mochitest/general/resource_timing_main_test.html:59:3
window.onload@tests/dom/tests/mochitest/general/resource_timing_main_test.html:80:1
3447 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/general/test_resource_timing.html | Performance.getEntriesByName() should return some results 
ok@tests/dom/tests/mochitest/general/resource_timing_main_test.html:55:3
window.onload@tests/dom/tests/mochitest/general/resource_timing_main_test.html:86:1
3448 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/general/test_resource_timing.html | An entry with the name 'http://mochi.test:8888/tests/image/test/mochitest/blue.png' should be available 
ok@tests/dom/tests/mochitest/general/resource_timing_main_test.html:55:3
checkEntries@tests/dom/tests/mochitest/general/resource_timing_main_test.html:196:1
window.onload@tests/dom/tests/mochitest/general/resource_timing_main_test.html:106:1

Not prettified yet but OK in my use-case.
Attachment #8598457 - Flags: feedback?(ted)
(Assignee)

Updated

3 years ago
Attachment #8598457 - Flags: feedback?(martijn.martijn)
(Assignee)

Updated

3 years ago
Attachment #8598457 - Flags: feedback?(jaws)
(Reporter)

Updated

3 years ago
Assignee: nobody → hiikezoe
(Reporter)

Comment 3

3 years ago
Comment on attachment 8598457 [details] [diff] [review]
A simple solution

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

Looks great to me. Perhaps you could indent the lines that contain the stack? That makes it a little clearer.
Attachment #8598457 - Flags: feedback?(martijn.martijn) → feedback+
Comment on attachment 8598457 [details] [diff] [review]
A simple solution

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

Looks fine to me, but I'm not in a position to really give an authoritative review of it.
Attachment #8598457 - Flags: feedback?(jaws) → feedback+
(Assignee)

Comment 5

3 years ago
Created attachment 8600247 [details] [diff] [review]
Output stack trace in failure case

:mwargers, :jaws, thank you for your feedbacks!

This patch adds 4 spaces to indent each line.

A example of the output:
Expected PASS, got FAIL
didn't expect 1, but got it
    SimpleTest.isnot@SimpleTest/SimpleTest.js:299:5
    isnot@dom/tests/mochitest/general/resource_timing_main_test.html:63:3
    secondCheck@dom/tests/mochitest/general/resource_timing_main_test.html:222:1

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb44ad6cbe6d
Attachment #8598457 - Attachment is obsolete: true
Attachment #8598457 - Flags: feedback?(ted)
Attachment #8600247 - Flags: review?(ted)
Attachment #8600247 - Flags: review?(ted) → review+
Sorry for the long review delay!
(Assignee)

Comment 8

3 years ago
Created attachment 8634417 [details] [diff] [review]
Output stack trace in failure case v2

Unrotted.

Thanks!
Attachment #8600247 - Attachment is obsolete: true
Attachment #8634417 - Flags: review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/19f222fd79fe
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.