Closed Bug 701632 Opened 8 years ago Closed 8 years ago

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

Categories

(Testing :: Mochitest, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
mozilla11

People

(Reporter: heycam, Assigned: heycam)

Details

Attachments

(1 file)

This can mess up log messages sometimes.
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");
Attachment #573756 - Flags: review?(jmaher) → review+
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.
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.
Followup, since I forgot to refresh my patch before applying/committing:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5c077ef3d19
https://hg.mozilla.org/mozilla-central/rev/80154fc0c582
https://hg.mozilla.org/mozilla-central/rev/e5c077ef3d19
Assignee: nobody → cam
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Component: BrowserTest → Mochitest
You need to log in before you can comment on or make changes to this bug.