[Webkit-unassigned] [Bug 53908] Get rid of code which writes test results from test_type/* classes.

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Dirk Pranke <dpranke at chromium.org>  2011-02-09 15:00:26 PST ---
(From update of attachment 81610)
View in context: https://bugs.webkit.org/attachment.cgi?id=81610&action=review

> Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py:81
> +

I feel like this should still be a transitional patch in the code and we can still do better. I think the fact that we're keeping test_types as a list of classes and iterating over them without knowing what they are is just making us have to create these big complicated if blocks. I think if we got rid of the lists of classes and just created the test type objects directly in single_test runner (at line 285) and then handled the failures directly it would be a lot more obvious what's going on. But, if that's too much to change at once, this looks okay as a stopgap.

> Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py:131
> +    def _write_into_file_at_path(self, file_path, contents, encoding):

You can get rid of the encoding parameter now, right? At which point, this whole function might be kind of unnecessary.

> Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py:173
> +    def copy_text(self, actual_text, expected_text):

I would probably call this "copy_text_files" to make it clear that you are copying both files. I was confused at first in write_test_result() because it looked like you were copying the value of actual_text to expected_text,

> Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py:208
> +    def copy_image(self, actual_image, expected_image):

smae thing ... copy_image_files()

> Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py:213
> +    def copy_image_hash(self, actual_image_hash, expected_image_hash):

copy_image_hashes()

> Tools/Scripts/webkitpy/layout_tests/test_types/image_diff.py:-84
> -            self._copy_image(filename, actual_driver_output.image, expected_image=None)

You can actually delete _copy_image() and _copy_hash() as well, right?

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