[Webkit-unassigned] [Bug 49431] Refactor TestTypeBase.compare_output().

[bugzilla-daemon at webkit.org]

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





--- Comment #3 from Dirk Pranke <dpranke at chromium.org>  2010-11-12 02:21:27 PST ---
(From update of attachment 73720)
View in context: https://bugs.webkit.org/attachment.cgi?id=73720&action=review

I've only gotten about half-way through so far, but it looks pretty good; definitely on the right track. I will finish reviewing tomorrow, as it's now 2:30 in the morning my time ;)

> WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:82
> +        self.image = image

Don't worry about lazy retrieval. Once the multi-processing patch lands, all of the image retrieval will only be done when it's needed.

> WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:241
> +                        image_hash_to_driver)

Ideally driver.run_test() returns a TestOutput. Then you can pull _get_driver_output_image() inside run_test(), and skip the write to the filesystem for DumpRenderTree-based implementations.

I actually also want there to be a TestInput class that is the sole argument passed to run_test(), but that's a separate patch :). Feel free to do that as well, if you like; it's less important.

> WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:249
> +                           test_output, end - start)

I think you can and should pull end-start into TestOutput.test_time (or something like that).

I think all of the fields in test_info should also be in TestOutput (and TestInput), so you don't need to pass both test_info and test_output. I.e. TestOutput subclasses TestInput which subclasses TestInfo, or something. Actually, I think once the multiprocessing stuff lands TestInput *is* TestInfo.

> WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:286
>          self._driver.stop()

We eventually need to flip these things so that SingleTestThread returns the TestOutput, not the TestResult, and that way process_output can become an instance method on TestShellThread instead of a static function (this allows us to pare off two more arguments to process output, and the routine actually becomes manageable). But that's a separate patch as well.

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