Bug 216944

Summary: Change dumpAsText to strip trailing spaces
Product: WebKit Reporter: Darin Adler <darin>
Component: Tools / TestsAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, glenn, jbedard, mkwst, sam, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=217041
Bug Depends on:    
Bug Blocks: 217016, 217018, 217019, 217020, 217022, 217024, 217027, 217029, 217030, 217035, 217036, 217037, 217038, 217039, 217040    
Attachments:
Description Flags
Patch
none
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
none
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch none

Description Darin Adler 2020-09-24 14:36:33 PDT 
We should change the dumpAsText machinery to strip trailing spaces.

We’re not getting any value for comparing them, this helps Alan Bujtas with a project that affects trailing space collapsing, there are a lot of irritating trailing spaces in WPT tests, and this even makes the expected.txt files a tiny, tiny bit smaller.

We’ll update all the expected.txt files with a script.
Comment 1 Darin Adler 2020-09-25 10:22:57 PDT 
I think I’ll do this in 4 steps:

1) Make run-webkit-tests ignore trailing space differences in expected.txt files.

2) Change DumpRenderTree and WebKitTestRunner to not dump any trailing spaces.

3) Do a global replace across all the expected.txt files to remove existing trailing spaces (probably won’t post a patch for review for this step).

4) Remove the code from (1), no need to ignore trailing space differences any more.

Anyone have a better idea?
Comment 2 Darin Adler 2020-09-25 10:27:37 PDT 
Here’s the command line I will use for step 3:

    find . -name '*-expected.txt' | xargs perl -i -pe 's/ +$//g'
Comment 3 Darin Adler 2020-09-25 12:06:51 PDT 
Created attachment 409716 [details]
Patch
Comment 4 zalan 2020-09-25 12:09:34 PDT 
Comment on attachment 409716 [details]
Patch

Thanks for doing this!
Comment 5 Darin Adler 2020-09-25 12:11:05 PDT 
Comment on attachment 409716 [details]
Patch

I have a slightly better version of this patch that I will land, but only after seeing no failures on EWS.
Comment 6 Darin Adler 2020-09-25 12:54:08 PDT Comment hidden (obsolete)
Comment 7 Darin Adler 2020-09-25 13:56:40 PDT Comment hidden (obsolete)
Comment 8 Darin Adler 2020-09-25 15:03:16 PDT 
Running into trouble with other things that write into the file like these:

CONSOLE MESSSAGE:
PROMPT:

Won’t be hard to fix.
Comment 9 Darin Adler 2020-09-25 16:09:36 PDT Comment hidden (obsolete)
Comment 10 Darin Adler 2020-09-25 18:33:06 PDT Comment hidden (obsolete)
Comment 11 Darin Adler 2020-09-26 08:35:46 PDT Comment hidden (obsolete)
Comment 12 Darin Adler 2020-09-26 13:43:32 PDT Comment hidden (obsolete)
Comment 13 Darin Adler 2020-09-26 14:58:34 PDT Comment hidden (obsolete)
Comment 14 Darin Adler 2020-09-26 15:16:37 PDT Comment hidden (obsolete)
Comment 15 Darin Adler 2020-09-26 15:44:59 PDT 
(In reply to Darin Adler from comment #2)
> Here’s the command line I will use for step 3:
> 
>     find . -name '*-expected.txt' | xargs perl -i -pe 's/ +$//g'

Fancier version that skips web archive files:

find . -name '*-expected.txt' | xargs awk '/^CONSOLE MESSAGE:/ { next }; /^<\?xml/ { nextfile }; { print FILENAME; nextfile }' | xargs perl -i -pe 's/ +$//g'
Comment 16 Darin Adler 2020-09-26 15:46:54 PDT 
Created attachment 409796 [details]
Patch
Comment 17 Darin Adler 2020-09-26 16:23:29 PDT 
Committed r267640: <https://trac.webkit.org/changeset/267640>
Comment 18 Radar WebKit Bug Importer 2020-09-26 16:24:16 PDT 
<rdar://problem/69637010>