1101039 - Add line numbers to assertions

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Martijn Wargers (dead)]

Attachment: 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.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: 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.

Comment 3

[Martijn Wargers (dead)]

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.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: 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

Comment 6

[Martijn Wargers (dead)]

Excellent, thanks! It didn't seem to work for mochitest-chrome, it seems:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-fb44ad6cbe6d/try-win64-debug/try_win8_64-debug_test-mochitest-other-bm109-tests1-windows-build689.txt.gz

Comment 7

[(not currently active) Ted Mielczarek]

Sorry for the long review delay!

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: Output stack trace in failure case v2

Unrotted.

Thanks!

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/19f222fd79fe

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/19f222fd79fe