[Webkit-unassigned] [Bug 55746] [NRWT] Clear output image file used by DRT to make sure the previous result should not be used.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Mar 6 20:13:17 PST 2011


https://bugs.webkit.org/show_bug.cgi?id=55746





--- Comment #5 from Hayato Ito <hayato at chromium.org>  2011-03-06 20:13:17 PST ---
(From update of attachment 84689)
View in context: https://bugs.webkit.org/attachment.cgi?id=84689&action=review

>>> Tools/Scripts/webkitpy/layout_tests/port/chromium.py:449
>>> +            self._port._filesystem.write_binary_file(png_path, '')
>> 
>> Isn't filesystem.remove more appropriate?
> 
> For consistency w/ the WebKitDriver implementation, we should *always* return '' if there is no actual PNG data, regardless of whether pixel tests are enabled or if it's a text-only test or if DRT crashed.
> 
> So, that said, I agree with Mihai that it would probably be clearer to write this as
> 
> if png_path and self._port._filesystem.exists(png_path):
>    self._port._filesystem.remove(png_path)
> 
> I think the extra write may actually slow things down a bit, too.
> 
> Then, you'll need to change _output_image to return '' if the file doesn't exist (currently it'll return None, which will trigger one of the other bugs).
> 
> We should actually add an assert in single_test_runner to enforce image being not None.

Thank you for the reviews.

I didn't notice that filesystem provides remove(path) function. Thank you.

I updated the patch to make sure the output image should be '', not None, if the file doesn't exist.
And I added some assertion in single_test_runner and ChromiumDriver to make sure image is not None.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list