Bug 57783

Created attachment 88114 [details] Patch

The bug is as follows: 1) The rebaseline tool writes a png results for win-xp 2) The rebaseline tool deletes the checksum result for win-xp, since it's not needed 3) The rebaseline tool tries to write a png result for win-vista. Since it matches the win-xp result, it does nothing. 4) The rebaseline tool writes a checksum result for win-vista, but since there's no win-xp checksum, it goes ahead and writes the file. The result is there's a .checksum file in win-vista that matches the .png file in win-xp. We shouldn't write the .checksum file. The patch fixes the bug by checking the fallback .png list rather than just checking for win-vista's .png.

(In reply to comment #2) Dirk tells me I described the fallback wrong. Here's a version without platforms but with directories instead: 1) The rebaseline tool writes a png results in chromium-win 2) The rebaseline tool deletes the checksum result for chromium-win, since it's not needed 3) The rebaseline tool tries to write a png result for chromium-win-vista. Since it matches the chromium-win result, it does nothing. 4) The rebaseline tool writes a checksum result for chromium-win-vista, but since there's no chromium-win checksum, it goes ahead and writes the file.

Comment on attachment 88114 [details] Patch LGTM. View in context: https://bugs.webkit.org/attachment.cgi?id=88114&action=review > Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:435 > + Nit: Can you pull this into a separate routine called png_for_checksum() or something like that to help clarify what's going on?

Created attachment 88118 [details] Patch

(In reply to comment #5) > Created an attachment (id=88118) [details] > Patch LGTM.

Comment on attachment 88114 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=88114&action=review >> Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:435 >> + png_fullpath = fs.join(all_baselines[0][0], all_baselines[0][1]) >> + > > Nit: Can you pull this into a separate routine called png_for_checksum() or something like that to help clarify what's going on? Done. Note: Code like this is why I hate returning tuples.

(In reply to comment #7) > (From update of attachment 88114 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=88114&action=review > > >> Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:435 > >> + png_fullpath = fs.join(all_baselines[0][0], all_baselines[0][1]) > >> + > > > > Nit: Can you pull this into a separate routine called png_for_checksum() or something like that to help clarify what's going on? > > Done. Note: Code like this is why I hate returning tuples. It looks like expected_baselines() could and probably should be rewritten to only return a list of full paths to the tests, and then maybe we provide another helper function like baseline_dir_for_test() or something. That would probably clean things up somewhat. I know this code used to be more convoluted in the past and it can probably be made a fair amount simpler.

Committed r82892: <http://trac.webkit.org/changeset/82892>