RESOLVED FIXED 64471
WIN: editing tests fail under NRWT because editing delegate callbacks aren't stripped 
https://bugs.webkit.org/show_bug.cgi?id=64471
Summary WIN: editing tests fail under NRWT because editing delegate callbacks aren't ...
Adam Roben (:aroben)
Reported 2011-07-13 11:23:46 PDT
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.
Attachments
Patch (3.00 KB, patch)
2011-10-24 15:47 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Patch (3.31 KB, patch)
2011-10-24 16:01 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Patch (3.23 KB, patch)
2011-10-24 16:56 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Adam Roben (:aroben)
Comment 1 2011-07-13 12:41:13 PDT
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)
Comment 2 2011-07-13 15:32:45 PDT
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)
Comment 3 2011-07-14 07:37:13 PDT
(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)
Comment 4 2011-10-24 15:47:18 PDT
Created attachment 112264 [details] Patch
Adam Roben (:aroben)
Comment 5 2011-10-24 15:51:58 PDT
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)
Comment 6 2011-10-24 15:54:41 PDT
Oh goodie. :)
Eric Seidel (no email)
Comment 7 2011-10-24 16:01:34 PDT
Created attachment 112268 [details] Patch
Adam Roben (:aroben)
Comment 8 2011-10-24 16:17:33 PDT
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)
Comment 9 2011-10-24 16:18:29 PDT
(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)
Comment 10 2011-10-24 16:56:58 PDT
Created attachment 112276 [details] Patch
WebKit Review Bot
Comment 11 2011-10-25 06:48:47 PDT
Comment on attachment 112276 [details] Patch Clearing flags on attachment: 112276 Committed r98340: <http://trac.webkit.org/changeset/98340>
WebKit Review Bot
Comment 12 2011-10-25 06:48:52 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.