1073074 - Browser chrome UI test output is broken

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Dave Townsend [:mossop]]

Here is the output displayed in the UI when running browser_fail.js:

Running chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js...
undefined | chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js | 
undefined | chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js | 
Stack trace:
chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js:test:2
chrome://mochikit/content/browser-test.js:Tester_execTest:659
chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest</<:556
null:null:0 NEXT ERROR
undefined | chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js | Got true, expected false
Stack trace:
chrome://mochikit/content/browser-test.js:test_is:793
chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js:test:3
chrome://mochikit/content/browser-test.js:Tester_execTest:659
chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest</<:556
null:null:0 NEXT ERROR
undefined | chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js | Didn't expect true, but got it
Stack trace:
chrome://mochikit/content/browser-test.js:test_isnot:797
chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js:test:4
chrome://mochikit/content/browser-test.js:Tester_execTest:659
chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest</<:556
null:null:0 NEXT ERROR
undefined | chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js | 
Stack trace:
chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js:test:5
chrome://mochikit/content/browser-test.js:Tester_execTest:659
chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest</<:556
null:null:0 NEXT ERROR
undefined | chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js | Got true, expected true
Stack trace:
chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js:test:6
chrome://mochikit/content/browser-test.js:Tester_execTest:659
chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest</<:556
null:null:0 NEXT ERROR
undefined | chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js | Didn't expect true, but got it
Stack trace:
chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js:test:7
chrome://mochikit/content/browser-test.js:Tester_execTest:659
chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest</<:556
null:null:0 NEXT ERROR
undefined | chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js | 
undefined | chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js | TEST-END | chrome://mochitests/content/browser/testing/mochitest/tests/browser/browser_fail.js | finished in 10 ms

Lots of undefineds and blank messages

Comment 1

[Dave Townsend [:mossop]]

This was broken by bug 886570

Comment 2

[Dave Townsend [:mossop]]

Attachment: patch

Comment 3

[Dave Townsend [:mossop]]

Also bug 1050251

Comment 4

[Dave Townsend [:mossop]]

Comment on attachment 8495413 [details] [diff] [review]
patch

Bug 886570 removed the result property from the testResult object. Bug 1073074 split the msg property into name and msg. This patch rebuilds those for display in the UI.

Comment 5

[Andrew Halberstadt [:ahal]]

Comment on attachment 8495413 [details] [diff] [review]
patch

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

Thanks for digging into this! I'd give this an r+, but the fact that you are manually building log strings makes me think Chris should take a look. Though if it's for display purposes only, I think it should be fine.

Comment 6

[Ahmed Kachkach [:akachkach]]

Maybe we can use the "StructuredFormatter" present in TestRunner.js?

Comment 7

[Dave Townsend [:mossop]]

(In reply to Ahmed Kachkach [:akachkach] from comment #6)
> Maybe we can use the "StructuredFormatter" present in TestRunner.js?

By all means file another bug to do that, for now though I'd like to just get this regression fixed.

Comment 8

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8495413 [details] [diff] [review]
patch

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

Right. The formatter in TestRunner would be the answer to this, but isn't applicable without more refactoring.

::: testing/mochitest/browser-harness.xul
@@ +176,5 @@
> +          }
> +          else if (t.pass) {
> +            classname += "passed";
> +            if (t.todo)
> +              result += "KNOWN_FAIL";

Might be "KNOWN-FAIL"

Comment 9

[Dave Townsend [:mossop]]

(In reply to Chris Manchester [:chmanchester] from comment #8)
> Comment on attachment 8495413 [details] [diff] [review]
> patch
> 
> Review of attachment 8495413 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Right. The formatter in TestRunner would be the answer to this, but isn't
> applicable without more refactoring.
> 
> ::: testing/mochitest/browser-harness.xul
> @@ +176,5 @@
> > +          }
> > +          else if (t.pass) {
> > +            classname += "passed";
> > +            if (t.todo)
> > +              result += "KNOWN_FAIL";
> 
> Might be "KNOWN-FAIL"

Good catch, fixed.

https://hg.mozilla.org/integration/fx-team/rev/df3fe2e4bf8f

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/df3fe2e4bf8f