Bug 86931 – LayoutTests: fast/js/resources/js-test-pre.js - shouldBeEqualToString fails on "

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Joshua Bell]

function shouldBeEqualToString(a, b)
{
  var unevaledString = '"' + b.replace(/\\/g, "\\\\").replace(/"/g, "\"").replace(/\n/g, "\\n").replace(/\r/g, "\\r") + '"';
  shouldBe(a, unevaledString);
}

That replace(/"/g, "\"") should be replace(/"/g, "\\"")

Or just replace that whole line with:

  var unevaledString = JSON.stringify(b);

[Joshua Bell]

Created attachment 143058 [details]
Patch

[Joshua Bell]

I'm expecting a handful of tests to 'asplode as a result of this change - anything that was relying on the broken escaping behavior of shouldBeEqualToString. Let's see what the bots say.

Alec, can you sanity check the IDB test change?

[Ojan Vafai]

Comment on attachment 143058 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=143058&action=review

> LayoutTests/fast/js/resources/js-test-pre.js:261
> +  var unevaledString = JSON.stringify(b);

On the one hand, this is simpler, on the other it means that if you're explicitly testing that you get a string and not and object that stringifies to the same thing, then this won't work. 

Can we add a typeof b == 'string' check?

[Alec Flett]

Glad to see this go in, I struggled to get this right and it drove me nuts - LGTM (agree on the typeof comment from ojan as well)

[Joshua Bell]

(In reply to comment #3)
> On the one hand, this is simpler, on the other it means that if you're explicitly testing that you get a string and not and object that stringifies to the same thing, then this won't work. 
> 
> Can we add a typeof b == 'string' check?

Agreed and will do. Shouldn't break anything either, as b.replace() was previously called without a String() coercion.

[WebKit Review Bot]

Comment on attachment 143058 [details]
Patch

Attachment 143058 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12749018

New failing tests:
storage/domstorage/complex-values.html
fast/dom/HTMLElement/class-list.html
fast/dom/HTMLElement/class-list-quirks.html
fast/dom/HTMLOutputElement/dom-settable-token-list.html

[WebKit Review Bot]

Created attachment 143068 [details]
Archive of layout-test-results from ec2-cr-linux-04

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-04  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Joshua Bell]

As expected...

(In reply to comment #6)
> New failing tests:
> storage/domstorage/complex-values.html

PASS->PASS, but output differs: contains an embedded NUL, was printed as \0 now printed as \u0000

> fast/dom/HTMLElement/class-list.html

PASS->PASS, but output differs: contains an embedded TAB, was printed as tab character, now printed as \t

> fast/dom/HTMLElement/class-list-quirks.html

PASS->PASS, but output differs: contains an embedded TAB, was printed as tab character, now printed as \t

> fast/dom/HTMLOutputElement/dom-settable-token-list.html

PASS->PASS, but output differs: contains an embedded TAB, was printed as tab character, now printed as \t

Will rebase these tests.

[Joshua Bell]

Created attachment 143069 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 143069 [details]
Patch for landing

Clearing flags on attachment: 143069

Committed r117806: <http://trac.webkit.org/changeset/117806>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.