WIN: editing tests fail under NRWT because editing delegate callbacks aren't stripped

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Adam Roben (:aroben)]

Apple's Windows port of DRT doesn't have support for editing delegate callbacks. (I don't think we even have an editing delegate API in WebKit1 on Windows.) old-run-webkit-tests strips out these callbacks before comparing actual and expected results. new-run-webkit-tests doesn't do this, which causes all editing tests to fail.

[Adam Roben (:aroben)]

The relevant code in ORWT is here: <http://trac.webkit.org/browser/trunk/Tools/Scripts/old-run-webkit-tests?rev=90125#L858>.

[Eric Seidel (no email)]

OK.  This is easy to fix.  The question is at what layer to fix it.

Is this only a hack for Apple Win, or do other ports want to use it?

SingleTestRunner already has a _get_normalized_output_text method which it uses to fix newlines before comparing results.

Port already has a compare_text method, which we could override in the WinPort to do this special compare, but then the "expected" results spit out would still have the editing callbacks. (are they supposed to?)

[Adam Roben (:aroben)]

(In reply to comment #2)
> Is this only a hack for Apple Win, or do other ports want to use it?

ORWT exposes a --[no-]strip-editing-callbacks, which defaults to true on Apple's Windows port. I don't know if anyone else uses that option.

> Port already has a compare_text method, which we could override in the WinPort to do this special compare, but then the "expected" results spit out would still have the editing callbacks. (are they supposed to?)

In ORWT, the expected results still contain editing callbacks (unless the expected results were generated on Windows, of course). This is a little confusing when looking at diffs, but of course removing the editing callbacks from the expected output would also be confusing.

I think either way is fine, but matching ORWT is probably a teensy bit better.

[Eric Seidel (no email)]

Created attachment 112264 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 112264 [details]
Patch

Note that ORWT doesn't do this for WebKit2 on Windows, only for WebKit1. NRWT should do the same.

[Eric Seidel (no email)]

Oh goodie. :)

[Eric Seidel (no email)]

Created attachment 112268 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 112268 [details]
Patch

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

> Tools/Scripts/webkitpy/layout_tests/port/win.py:84
> +        deletate_regxp = re.compile("^EDITING DELEGATE: .*?\n", re.MULTILINE)

Mmmmmm, deletate.

> Tools/Scripts/webkitpy/layout_tests/port/win.py:88
> +        # FIXME: Why this is != is unclear from the design.  This matches base.Port
> +        return expected_text != actual_text

I'm not sure what this FIXME is saying. What do you think we should be using instead of != ?

[Eric Seidel (no email)]

(In reply to comment #8)
> (From update of attachment 112268 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=112268&action=review
> 
> > Tools/Scripts/webkitpy/layout_tests/port/win.py:84
> > +        deletate_regxp = re.compile("^EDITING DELEGATE: .*?\n", re.MULTILINE)
> 
> Mmmmmm, deletate.

Will fix.

> > Tools/Scripts/webkitpy/layout_tests/port/win.py:88
> > +        # FIXME: Why this is != is unclear from the design.  This matches base.Port
> > +        return expected_text != actual_text
> 
> I'm not sure what this FIXME is saying. What do you think we should be using instead of != ?

I'll just remove the fixme.  I would expect it to say ==.

[Eric Seidel (no email)]

Created attachment 112276 [details]
Patch

[WebKit Review Bot]

Comment on attachment 112276 [details]
Patch

Clearing flags on attachment: 112276

Committed r98340: <http://trac.webkit.org/changeset/98340>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.