Bug 232643 - UnicodeDecodeError in write_reftest copying a non-UTF8 expected result file
Summary: UnicodeDecodeError in write_reftest copying a non-UTF8 expected result file
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-11-02 15:21 PDT by Tim Horton
Modified: 2021-11-02 20:38 PDT (History)
9 users (show)

See Also:


Attachments
Patch (2.13 KB, patch)
2021-11-02 15:25 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (2.33 KB, patch)
2021-11-02 18:53 PDT, Tim Horton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2021-11-02 15:21:35 PDT
UnicodeDecodeError in write_reftest copying a non-UTF8 expected result file
Comment 1 Tim Horton 2021-11-02 15:25:08 PDT
Created attachment 443139 [details]
Patch
Comment 2 Alexey Proskuryakov 2021-11-02 15:40:31 PDT
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?
Comment 3 Tim Horton 2021-11-02 18:20:20 PDT
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 :)
Comment 4 Tim Horton 2021-11-02 18:53:30 PDT
Created attachment 443159 [details]
Patch
Comment 5 EWS 2021-11-02 20:37:59 PDT
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].
Comment 6 Radar WebKit Bug Importer 2021-11-02 20:38:17 PDT
<rdar://problem/84959425>