RESOLVED FIXED 216944
Change dumpAsText to strip trailing spaces
https://bugs.webkit.org/show_bug.cgi?id=216944
Summary Change dumpAsText to strip trailing spaces
Darin Adler
Reported 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.
Attachments
Patch (5.03 KB, patch)
2020-09-25 12:06 PDT, Darin Adler
no flags
Patch (7.97 KB, patch)
2020-09-25 12:54 PDT, Darin Adler
ews-feeder: commit-queue-
Patch (8.10 KB, patch)
2020-09-25 13:56 PDT, Darin Adler
ews-feeder: commit-queue-
Patch (21.52 KB, patch)
2020-09-25 16:09 PDT, Darin Adler
ews-feeder: commit-queue-
Patch (23.53 KB, patch)
2020-09-25 18:33 PDT, Darin Adler
ews-feeder: commit-queue-
Patch (23.61 KB, patch)
2020-09-26 08:35 PDT, Darin Adler
ews-feeder: commit-queue-
Patch (25.59 KB, patch)
2020-09-26 13:43 PDT, Darin Adler
no flags
Patch (26.07 KB, patch)
2020-09-26 14:58 PDT, Darin Adler
ews-feeder: commit-queue-
Patch (26.22 KB, patch)
2020-09-26 15:16 PDT, Darin Adler
ews-feeder: commit-queue-
Patch (26.22 KB, patch)
2020-09-26 15:46 PDT, Darin Adler
no flags
Darin Adler
Comment 1 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?
Darin Adler
Comment 2 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'
Darin Adler
Comment 3 2020-09-25 12:06:51 PDT
zalan
Comment 4 2020-09-25 12:09:34 PDT
Comment on attachment 409716 [details] Patch Thanks for doing this!
Darin Adler
Comment 5 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.
Darin Adler
Comment 6 2020-09-25 12:54:08 PDT Comment hidden (obsolete)
Darin Adler
Comment 7 2020-09-25 13:56:40 PDT Comment hidden (obsolete)
Darin Adler
Comment 8 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.
Darin Adler
Comment 9 2020-09-25 16:09:36 PDT Comment hidden (obsolete)
Darin Adler
Comment 10 2020-09-25 18:33:06 PDT Comment hidden (obsolete)
Darin Adler
Comment 11 2020-09-26 08:35:46 PDT Comment hidden (obsolete)
Darin Adler
Comment 12 2020-09-26 13:43:32 PDT Comment hidden (obsolete)
Darin Adler
Comment 13 2020-09-26 14:58:34 PDT Comment hidden (obsolete)
Darin Adler
Comment 14 2020-09-26 15:16:37 PDT Comment hidden (obsolete)
Darin Adler
Comment 15 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'
Darin Adler
Comment 16 2020-09-26 15:46:54 PDT
Darin Adler
Comment 17 2020-09-26 16:23:29 PDT
Radar WebKit Bug Importer
Comment 18 2020-09-26 16:24:16 PDT
Note You need to log in before you can comment on or make changes to this bug.