613516 - (greenpocalypse) xpcshell dump doesn't print newlines

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Benjamin Smedberg]

dump("\n") in xpcshell used to print newlines to stdout. Now it prints "\n". This is a regression from bug 607695 and is an unwanted change in behavior because it makes reading xpcshell test output really hard.

jorendorff says we're using JS_FileEscapedString but really shouldn't.

Comment 2

[Dave Townsend [:mossop]]

This is making it really hard to work on testcases for blockers

Comment 3

[Igor Bukanov]

Attachment: v1

Comment 4

[Igor Bukanov]

Comment on attachment 492054 [details] [diff] [review]
v1

The patch restore dump to print the string with no escapes.

Comment 5

[Josh Matthews [:jdm]]

Can we land this asap?  It's really annoying when it bites.

Comment 6

[:Margaret Leibovic]

http://hg.mozilla.org/mozilla-central/rev/650ad3cd7e52

Comment 7

[(no longer active)]

I backed this out because of the xpcshell test orange:

http://hg.mozilla.org/mozilla-central/rev/e96da44dcae1

Sample log:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1291069330.1291071106.6580.gz

Comment 8

[Phil Ringnalda (:philor)]

That's actually one of two failures, there's also http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1291070111.1291071714.9376.gz on Mac debug which is also failing on TraceMonkey, unlike the other which is apparently some not-yet-merged dependency.

Comment 9

[Shawn Wilsher :sdwilsh]

(In reply to comment #7)
> I backed this out because of the xpcshell test orange:
Just to be sure, this isn't a case of tinderbox print now actually catching something that was orange before, right?

Comment 10

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #9)
> (In reply to comment #7)
> > I backed this out because of the xpcshell test orange:
> Just to be sure, this isn't a case of tinderbox print now actually catching
> something that was orange before, right?

Might be. The failure in Phil's log should have been failing since it landed.
http://mxr.mozilla.org/mozilla-central/source/netwerk/test/unit/test_bug468426.js#64

> do_timeout(1, "triggerNextTest();");

should just be

> do_timeout(1, triggerNextTest);

Comment 11

[Shawn Wilsher :sdwilsh]

I've closed the tree.  We need to fix any xpcshell orange from this landing again as it looks like it is actually failure being caught by tinderbox now that all output from dump is not on one line.  That means the tree stays closed until these oranges are resolved.  Paul thinks he has a fix for the only issue that we saw last time (comment 10), and he's going to land that and this patch again.

Is it possible for us to get a test for this?

Comment 12

[Shawn Wilsher :sdwilsh]

Given recent information, I think this actually needs to block beta 8.

Comment 13

[(no longer active)]

Bumping the priority to blocker, as this has caused us to keep mozilla-central closed.

Comment 14

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Bustage fix

this in combination with relanding the patch for this bug passes xpcshell tests for me (linux 64 debug).

Also, it seems that the broken dump has caused us to not have any orange xpcshell test on tinderbox (except for crashes, timeouts).

Comment 15

[(no longer active)]

Landed:

http://hg.mozilla.org/mozilla-central/rev/9231797ba106
http://hg.mozilla.org/mozilla-central/rev/d1fe3a682617

Will keep the tree closed until we get green runs after this.

Comment 16

[(no longer active)]

I landed a couple of followup fixes:

http://hg.mozilla.org/mozilla-central/rev/00fb5565fc30
http://hg.mozilla.org/mozilla-central/rev/e57dc7264ff5