701632 - browser-test.js sometimes calls dump without a newline on the end

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Cameron McCormack (:heycam)]

This can mess up log messages sometimes.

Comment 1

[Cameron McCormack (:heycam)]

Attachment: Add some newlines to dump() calls in browser-test.js.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 573756 [details] [diff] [review]
Add some newlines to dump() calls in browser-test.js.

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

r=me with this one issue addressed.

::: testing/mochitest/browser-test.js
@@ +187,5 @@
>        var msg = "Console message: " + aConsoleMessage.message;
>        if (this.currentTest)
>          this.currentTest.addResult(new testMessage(msg));
>        else
> +        this.dumper.dump("TEST-INFO | (browser-test.js) | " + msg + "\n");

would we be adding an extra \n in some cases.  I would think something similar to:
this.dumper.dump("TEST-INFO | (browser-test.js) | " + msg.strip('\n') + "\n");

Comment 3

[:Gavin Sharp [email: [e-mail]]]

aConsoleMessage.message doesn't normally include a trailing newline, AFAIK. Even if it does, an occasional extra newline seems better than stripping _all_ newlines from the message.

Comment 4

[Cameron McCormack (:heycam)]

I think Gavin is probably right regarding console messages not typically including newlines, but I guess it can't hurt to do `msg.replace(/\n+$/, "") + "\n"` there.

Comment 5

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/80154fc0c582

Comment 6

[Cameron McCormack (:heycam)]

Followup, since I forgot to refresh my patch before applying/committing:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5c077ef3d19

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/80154fc0c582
https://hg.mozilla.org/mozilla-central/rev/e5c077ef3d19