Bug 58402

Created attachment 89313 [details] Patch

I'll send a note to webkit-dev before landing this.

Feel free to suggest other reviewers.

Comment on attachment 89313 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=89313&action=review > Tools/Scripts/old-run-webkit-tests:936 > + if (-f $expectedChecksumPath) { > + unlink $expectedChecksumPath; > + } This seems strange. People are going to have files disappearing from their working trees without any explanation. I think it would be a lot better to just do a one-time delete of all .checksum files in the WebKit tree and commit that separately.

(In reply to comment #4) > This seems strange. People are going to have files disappearing from their working trees without any explanation. I think it would be a lot better to just do a one-time delete of all .checksum files in the WebKit tree and commit that separately. I said I would send a note to webkit-dev before landing this :) But if you prefer, I can update the existing pngs and remove the .checksum files first. The downside is there will be a window of time between the patches (or patches created before the .checksum delete and landed after) where people will be checking in .checksums after I've deleted all of them. I guess I can just do a second pass a week later.

Seems like the steps could be: 1) Embed checksums in all existing PNGs 2) Change ORWT not to read or write .checksum files 3) Delete all .checksum files These could each be done separately.

(In reply to comment #6) > Seems like the steps could be: > > 1) Embed checksums in all existing PNGs > 2) Change ORWT not to read or write .checksum files > 3) Delete all .checksum files > > These could each be done separately. That sounds fine to me. This is part of step 2 (the read part is done). I'll come back and r? this once step 1 is done.

*** Bug 57575 has been marked as a duplicate of this bug. ***

Created attachment 90566 [details] Patch

Comment on attachment 90566 [details] Patch The ORWT changes look good to me. Someone more familiar with NRWT should review that part.

(In reply to comment #6) > 1) Embed checksums in all existing PNGs > 2) Change ORWT not to read or write .checksum files > 3) Delete all .checksum files (1) is done. This is (2). I'll send a note to webkit-dev after landing this. It'll be harmless if people continue to check in .checksum files for a few days. They'll just be ignored.

Created attachment 90567 [details] add comment back to ORWT

Comment on attachment 90567 [details] add comment back to ORWT View in context: https://bugs.webkit.org/attachment.cgi?id=90567&action=review > Tools/Scripts/webkitpy/layout_tests/layout_package/single_test_runner.py:149 > if self._options.pixel_tests and driver_output.image_hash: maybe this check should be driver_output.image instead of driver_output.image_hash now?

Committed r84544: <http://trac.webkit.org/changeset/84544>

(In reply to comment #13) > (From update of attachment 90567 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=90567&action=review > > > Tools/Scripts/webkitpy/layout_tests/layout_package/single_test_runner.py:149 > > if self._options.pixel_tests and driver_output.image_hash: > > maybe this check should be driver_output.image instead of driver_output.image_hash now? I switched this before committing. There's a lot of code in NRWT that we can update/delete now, but I'll do that in a follow up patch.