| Summary: | UnicodeDecodeError in write_reftest copying a non-UTF8 expected result file | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||
| Component: | Tools / Tests | Assignee: | Tim Horton <thorton> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ap, dino, ews-watchlist, glenn, gsnedders, jbedard, simon.fraser, webkit-bug-importer, wenson_hsieh | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Tim Horton
2021-11-02 15:21:35 PDT
Created attachment 443139 [details]
Patch
Comment on attachment 443139 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=443139&action=review > Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py:208 > - self._write_text_file(dst_filepath, fs.read_text_file(src_filepath)) > + fs.copyfile(src_filepath, dst_filepath) I wanted to say r+, but then I did an svn blame and found that r122505 changed this from copyfile to read/write. There was more refactoring there, so this is probably still ok, but I didn't take the time to understand why. Would you mind figuring out and explaining it? Ooh, good catch, it was so that the code to make the enclosing directory would be run. We should do that in this case too (but in this function directly instead of by re-using _write_text_file). I don't think Dirk's reasoning that reftests should use the same mechanism stands up, because we can (??) ensure that the test output is UTF-8 (since it is the output of WebKit), but can't do the same for the input. Will fix, thank you for the observation :) Created attachment 443159 [details]
Patch
Committed r285198 (243824@main): <https://commits.webkit.org/243824@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 443159 [details]. |