[Webkit-unassigned] [Bug 70484] [WK2] Enable pixel test on WK2 if expected.png exists

[bugzilla-daemon at webkit.org]

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


Balazs Kelemen <kbalazs at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #111744|1                           |0
        is obsolete|                            |




--- Comment #3 from Balazs Kelemen <kbalazs at webkit.org>  2011-10-20 09:28:13 PST ---
(From update of attachment 111744)
View in context: https://bugs.webkit.org/attachment.cgi?id=111744&action=review

In general the concept is good but I'm not happy with the semantics of the new switch (wk2_pixel_test).

> Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:100
> +        if self._port.driver_name() == "WebKitTestRunner" and self._port.get_option('wk2_pixel_test'):
> +            return self._should_run_pixel_test

1. wk2_pixel_test does not express the concept. I would use a separate switch, something like --skip-pixel-test-if-no-baseline. It would be valid only with -p (alias --pixel-tests), so it should be ignored with a warning otherwise
2. it's ok to only support it with WTR, but in that case we should die with error message if it is used with DRT

> Tools/WebKitTestRunner/TestInvocation.cpp:118
> +void TestInvocation::setIsWK2PixelTest(const std::string& expectedPixelHash)
> +{
> +    if (!expectedPixelHash.length())
> +        return;
> +    m_dumpPixels = true;
> +    m_expectedPixelHash = expectedPixelHash;
> +}

According to 1, you could do it in setIsPixelTest. You should still have an extra bool if you want to check that there is no empty hash when running without the --skip... switch, but I don't thing it is really necessary.

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