Bug 38027 - check-webkit-style complains about non-utf8 data in layout test result
Summary: check-webkit-style complains about non-utf8 data in layout test result
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2010-04-22 20:16 PDT by Tony Chang
Modified: 2010-04-24 00:03 PDT (History)
6 users (show)

See Also:

Patch (23.22 KB, patch)
2010-04-23 19:10 PDT, Eric Seidel (no email)
abarth: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Chang 2010-04-22 20:16:12 PDT
The error is here:

I have non-utf8 data in a layout test result (0xa0, a whitespace character).  I think this is quite common.

I'm not sure what the right fix is here.  You could also imagine layout tests that are not utf8.
Comment 1 Eric Seidel (no email) 2010-04-23 13:00:07 PDT
The right fix is to treat patch files a binary data, since I now realize they have no encoding.

I'll fix the callers and post a patch.
Comment 2 Eric Seidel (no email) 2010-04-23 14:29:51 PDT
This seems wrong.  I agree with you that patch files are binary.  But in this patch, it appears the -expected.txt file is not utf-8?

All output from DumpRenderTree is utf-8 as far as I know.  So how did you generate this expected.txt file and why wouldn't it be utf-8?

Comment 3 Eric Seidel (no email) 2010-04-23 15:31:49 PDT
I have a patch to fix our handling of patch files.  Finishing testing now.
Comment 4 Eric Seidel (no email) 2010-04-23 19:10:51 PDT
Created attachment 54210 [details]
Comment 5 Adam Barth 2010-04-23 20:15:55 PDT
Comment on attachment 54210 [details]

 +          # as we're passing --name-status which does not output any data.
This comment seems gratuitously different than the others.  Also, technically, file names (on unix) might not be UTF8 because they are just bytes.  I don't think we should worry about that here though.  Just more background for the trail of tears.

Comment 6 WebKit Commit Bot 2010-04-23 21:26:04 PDT
Comment on attachment 54210 [details]

Rejecting patch 54210 from commit-queue.

Failed to run "['WebKitTools/Scripts/test-webkitpy']" exit_code: 1
Last 500 characters of output:
cripts/webkitpy/common/prettypatch.py", line 42, in pretty_diff_file
    pretty_diff = self.pretty_diff(diff)
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/common/prettypatch.py", line 60, in pretty_diff
    return self._executive.run_command(args, input=diff, decode_output=False)
TypeError: run_command() got an unexpected keyword argument 'decode_output'

Ran 385 tests in 5.007s

FAILED (errors=3)

Full output: http://webkit-commit-queue.appspot.com/results/1856073
Comment 7 Eric Seidel (no email) 2010-04-24 00:03:52 PDT
Committed r58210: <http://trac.webkit.org/changeset/58210>