Attachment 154101 Details for Bug 91754 – proposed patch, test included

[patch] proposed patch, test included

bug-91754-20120724194142.patch (text/plain), 25.95 KB, created by Balazs Kelemen on 2012-07-24 10:41:59 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Balazs Kelemen

Created: 2012-07-24 10:41:59 PDT

Size: 25.95 KB

patch

obsolete

>Subversion Revision: 123453
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 00d9699439de7ba0f0ebf42bd81a80d69cebd789..15424c6f3e66a510b62e80c61a58fd7641fed153 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,81 @@
>+2012-07-24  Balazs Kelemen  <kbalazs@webkit.org>
>+
>+        [NRWT] should have a way to restrict pixel tests for individual directories
>+        https://bugs.webkit.org/show_bug.cgi?id=91754
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added --pixel-test-directories option to nrwt. If specified together
>+        with --pixel-tests, and if the platform supports this option, only tests
>+        from these directories will be executed as pixel tests. Furthermore, platforms
>+        can have a default set of pixel test directories. To support the feature
>+        the test harness have to be able to inform the driver whether to dump pixels
>+        for each tests. For this purpose I changed WTR and the Qt DRT to handle a
>+        --pixel-test argument from the standard output and made NRWT use this argument
>+        with these drivers.
>+
>+        * DumpRenderTree/qt/DumpRenderTreeQt.cpp:
>+        (WebCore::DumpRenderTree::DumpRenderTree):
>+        (WebCore::DumpRenderTree::open):
>+        (WebCore::DumpRenderTree::processLine):
>+        (WebCore::DumpRenderTree::setDumpPixelsForAllTests):
>+        (WebCore::DumpRenderTree::dump):
>+        * DumpRenderTree/qt/DumpRenderTreeQt.h:
>+        (DumpRenderTree):
>+        * DumpRenderTree/qt/main.cpp:
>+        (main):
>+        Handle the --pixel-test argument.
>+
>+        * Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:
>+        (SingleTestRunner._should_fetch_expected_checksum):
>+        (SingleTestRunner._overwrite_baselines):
>+        (SingleTestRunner._compare_output):
>+        * Scripts/webkitpy/layout_tests/controllers/worker.py:
>+        (Worker._update_test_input):
>+        * Scripts/webkitpy/layout_tests/port/base.py:
>+        (Port.make_relative_test_filename):
>+        (Port.relative_test_filename):
>+        (Port.supports_switching_pixel_tests_per_test): Ports can override this to inform
>+        the harness that --pixel-test argument is supported. True for WTR, otherwise calls
>+        private version that can be overridden by the actual platform. Only true with WTR
>+        and Qt DRT currently. This supposed to exist only temporary until there are ports
>+        that doesn't support the feature in their DRT's.
>+        (Port):
>+        (Port._supports_switching_pixel_tests_per_test):
>+        (Port.should_run_as_pixel_test):
>+        (Port._in_default_pixel_test_directory): Ports can override this to define their
>+        default set of pixel tests directories.
>+        * Scripts/webkitpy/layout_tests/port/driver.py:
>+        (Driver.cmd_line):
>+        (Driver._command_from_driver_input):  Pass --pixel-test if it is supported and pixel
>+        tests are enabled.
>+        * Scripts/webkitpy/layout_tests/port/qt.py:
>+        (QtPort._supports_switching_pixel_tests_per_test):
>+        (QtPort._in_default_pixel_test_directory):
>+        (QtPort._default_pixel_test_directories): Only compositing will be our default pixel
>+        test directory for the time being.
>+        * Scripts/webkitpy/layout_tests/port/test.py:
>+        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
>+        (_set_up_derived_options):
>+        (_listarg_callback):
>+        (parse_args):
>+        Handle the --pixel-test-directories option. optparse don't support list arguments
>+        so I implemented it with a callback. The argument can be a simple directory or a
>+        string (in apostrophes) containing a space separated list of directories.
>+
>+        * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:
>+        (MainTest.test_run_singly_actually_runs_tests):
>+        (MainTest.test_pixel_test_directories):
>+        Added integration test for the feature.
>+
>+        * WebKitTestRunner/TestController.cpp:
>+        (WTR::TestController::TestController):
>+        (WTR::TestController::initialize):
>+        (WTR::TestController::runTest):
>+        * WebKitTestRunner/TestController.h:
>+        (TestController):
>+        Handle the --pixel-test argument.
>+
> 2012-07-24  Pierre Rossi  <pierre.rossi@gmail.com>
> 
>         [Qt] Enable touch slider when touch events are enabled
>diff --git a/Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp b/Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp
>index 12d659a5193e39709c194e5716e47866de0f0bb8..8212bf1e7c43512832394b561fe465594c881c80 100755
>--- a/Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp
>+++ b/Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp
>@@ -388,7 +388,7 @@ WebViewGraphicsBased::WebViewGraphicsBased(QWidget* parent)
> }
> 
> DumpRenderTree::DumpRenderTree()
>-    : m_dumpPixels(false)
>+    : m_dumpPixelsForAllTests(false)
>     , m_stdin(0)
>     , m_enableTextOutput(false)
>     , m_standAloneMode(false)
>@@ -606,7 +606,7 @@ void DumpRenderTree::open(const QUrl& url)
> 
>     if (isDumpAsTextTest(url)) {
>         layoutTestController()->dumpAsText();
>-        setDumpPixels(false);
>+        setDumpPixelsForAllTests(false);
>     }
> 
>     if (isGlobalHistoryTest(url))
>@@ -692,14 +692,24 @@ void DumpRenderTree::processLine(const QString &input)
> {
>     QString line = input;
> 
>+    m_dumpPixelsForCurrentTest = false;
>     m_expectedHash = QString();
>-    if (m_dumpPixels) {
>-        // single quote marks the pixel dump hash
>-        int i = line.indexOf('\'');
>-        if (i > -1) {
>-            m_expectedHash = line.mid(i + 1, line.length());
>-            line.remove(i, line.length());
>+    // single quote marks the pixel dump hash
>+    int indexOfFirstSeparator = line.indexOf('\'');
>+    int indexOfSecondSeparator = line.indexOf('\'', indexOfFirstSeparator);
>+    if (indexOfFirstSeparator > -1) {
>+        int indexOfPixelHash = indexOfFirstSeparator + 1;
>+
>+        // NRWT passes --pixel-test if we should dump pixels for the test.
>+        const QString expectedArg(QLatin1String("--pixel-test"));
>+        QString argTest = line.mid(indexOfFirstSeparator + 1, expectedArg.length());
>+        if (argTest == expectedArg) {
>+            m_dumpPixelsForCurrentTest = true;
>+            indexOfPixelHash = indexOfSecondSeparator == -1 ? -1 : indexOfSecondSeparator + 1;
>         }
>+        if (indexOfPixelHash != -1 && indexOfPixelHash < line.size())
>+            m_expectedHash = line.mid(indexOfPixelHash);
>+        line.remove(indexOfFirstSeparator, line.length());
>     }
> 
>     if (line.startsWith(QLatin1String("http:"))
>@@ -731,9 +741,9 @@ void DumpRenderTree::processLine(const QString &input)
>     fflush(stdout);
> }
> 
>-void DumpRenderTree::setDumpPixels(bool dump)
>+void DumpRenderTree::setDumpPixelsForAllTests(bool dump)
> {
>-    m_dumpPixels = dump;
>+    m_dumpPixelsForAllTests = dump;
> }
> 
> void DumpRenderTree::closeRemainingWindows()
>@@ -951,7 +961,7 @@ void DumpRenderTree::dump()
>     fputs("#EOF\n", stdout);
>     fputs("#EOF\n", stderr);
> 
>-    if (m_dumpPixels && !m_controller->shouldDumpAsText()) {
>+    if ((m_dumpPixelsForAllTests || m_dumpPixelsForCurrentTest) && !m_controller->shouldDumpAsText()) {
>         QImage image;
>         if (!m_controller->isPrinting()) {
>             image = QImage(m_page->viewportSize(), QImage::Format_ARGB32);
>diff --git a/Tools/DumpRenderTree/qt/DumpRenderTreeQt.h b/Tools/DumpRenderTree/qt/DumpRenderTreeQt.h
>index d3928bc99faf153d97872aea1e5fe1e1a37d8d39..c4f92983f915cca6a8898adefb562c8d2dd91da0 100644
>--- a/Tools/DumpRenderTree/qt/DumpRenderTreeQt.h
>+++ b/Tools/DumpRenderTree/qt/DumpRenderTreeQt.h
>@@ -82,7 +82,7 @@ public:
>     void setGraphicsBased(bool flag) { m_graphicsBased = flag; }
>     bool isGraphicsBased() { return m_graphicsBased; }
> 
>-    void setDumpPixels(bool);
>+    void setDumpPixelsForAllTests(bool);
> 
>     void closeRemainingWindows();
>     void resetToConsistentStateBeforeTesting(const QUrl&);
>@@ -142,7 +142,8 @@ private:
>     QString dumpFrameScrollPosition(QWebFrame* frame);
>     LayoutTestController *m_controller;
> 
>-    bool m_dumpPixels;
>+    bool m_dumpPixelsForAllTests;
>+    bool m_dumpPixelsForCurrentTest;
>     QString m_expectedHash;
>     QStringList m_standAloneModeTestList;
> 
>diff --git a/Tools/DumpRenderTree/qt/main.cpp b/Tools/DumpRenderTree/qt/main.cpp
>index 8188f1816ffaba050ffa94022263b4d971f21a1a..22fbc75c395a10f4024e1755005b2d3be3d67f92 100644
>--- a/Tools/DumpRenderTree/qt/main.cpp
>+++ b/Tools/DumpRenderTree/qt/main.cpp
>@@ -192,7 +192,7 @@ int main(int argc, char* argv[])
> 
>     int index = args.indexOf(QLatin1String("--pixel-tests"));
>     if (index != -1) {
>-        dumper.setDumpPixels(true);
>+        dumper.setDumpPixelsForAllTests(true);
>         args.removeAt(index);
>     }
> 
>diff --git a/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py b/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py
>index fa07e88ff559b427ac99148a748c6d31db467f9f..acd5677e9866302e4efe417ca1209ee36192d847 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py
>@@ -78,10 +78,7 @@ class SingleTestRunner(object):
>                                  self._port.expected_audio(self._test_name))
> 
>     def _should_fetch_expected_checksum(self):
>-        if not self._should_run_pixel_test:
>-            return False
>-        return (self._options.pixel_tests and
>-                not (self._options.new_baseline or self._options.reset_results))
>+        return self._should_run_pixel_test and not (self._options.new_baseline or self._options.reset_results)
> 
>     def _driver_input(self):
>         # The image hash is used to avoid doing an image dump if the
>@@ -153,7 +150,7 @@ class SingleTestRunner(object):
>         location = self.VERSION_DIR if self._options.add_platform_exceptions else self.UPDATE
>         self._save_baseline_data(driver_output.text, '.txt', location)
>         self._save_baseline_data(driver_output.audio, '.wav', location)
>-        if self._options.pixel_tests:
>+        if self._should_run_pixel_test:
>             self._save_baseline_data(driver_output.image, '.png', location)
> 
>     def _save_baseline_data(self, data, extension, location):
>@@ -222,7 +219,7 @@ class SingleTestRunner(object):
> 
>         failures.extend(self._compare_text(driver_output.text, expected_driver_output.text))
>         failures.extend(self._compare_audio(driver_output.audio, expected_driver_output.audio))
>-        if self._options.pixel_tests:
>+        if self._should_run_pixel_test:
>             failures.extend(self._compare_image(driver_output, expected_driver_output))
>         return TestResult(self._test_name, failures, driver_output.test_time, driver_output.has_stderr())
> 
>diff --git a/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py b/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py
>index 378826c51f8e09df65a2800d75fc65e2ba1ae58b..0dc0983d6c74b237a30c7e3cd18b758ed87e75f9 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py
>@@ -88,13 +88,7 @@ class Worker(object):
>             test_input.reference_files = self._port.reference_files(test_input.test_name)
>         if test_input.reference_files:
>             test_input.should_run_pixel_test = True
>-        elif self._options.pixel_tests:
>-            if self._options.skip_pixel_test_if_no_baseline:
>-                test_input.should_run_pixel_test = bool(self._port.expected_image(test_input.test_name))
>-            else:
>-                test_input.should_run_pixel_test = True
>-        else:
>-            test_input.should_run_pixel_test = False
>+        test_input.should_run_pixel_test = self._port.should_run_as_pixel_test(test_input)
> 
>     def _run_test(self, test_input):
>         self._update_test_input(test_input)
>diff --git a/Tools/Scripts/webkitpy/layout_tests/port/base.py b/Tools/Scripts/webkitpy/layout_tests/port/base.py
>index d79143189d040a2f436a0895d92319d90e4682a9..7d0a25786f3694b5fb52c08945b095ae54aa5540 100755
>--- a/Tools/Scripts/webkitpy/layout_tests/port/base.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/port/base.py
>@@ -738,12 +738,26 @@ class Port(object):
> 
>         return self._filesystem.join(self._webkit_baseline_path(port_name), 'TestExpectations')
> 
>+    def make_relative_test_filename(self, filename):
>+        """Returns a test_name a relative unix-style path for a filename under the LayoutTests
>+        directory. filename should be an absolute or relative path of a test directory or a test file.
>+        Relative paths can be started either with the test directory ('LayoutTests') or one level below.
>+        Returns None if filename does not satisfy these conditions."""
>+
>+        relative_path = filename
>+        if self._filesystem.isabs(filename):
>+            if not filename.startswith(self.layout_tests_dir()):
>+                return None
>+            relative_path = self.relative_test_filename(filename)
>+        elif filename.startswith(self._filesystem.basename(self.layout_tests_dir())):
>+            relative_path = filename[len(self._filesystem.basename(self.layout_tests_dir())) + 1:]
>+
>+        return relative_path if self._filesystem.exists(self._filesystem.join(self.layout_tests_dir(), relative_path)) else None
>+
>     def relative_test_filename(self, filename):
>         """Returns a test_name a realtive unix-style path for a filename under the LayoutTests
>         directory. Filenames outside the LayoutTests directory should raise
>         an error."""
>-        # Ports that run on windows need to override this method to deal with
>-        # filenames with backslashes in them.
>         assert filename.startswith(self.layout_tests_dir()), "%s did not start with %s" % (filename, self.layout_tests_dir())
>         return filename[len(self.layout_tests_dir()) + 1:]
> 
>@@ -1249,6 +1263,29 @@ class Port(object):
>                 return suite.args
>         return []
> 
>+    def supports_switching_pixel_tests_per_test(self):
>+        if self.get_option('webkit_test_runner'):
>+            return True
>+        return self._supports_switching_pixel_tests_per_test()
>+
>+    def _supports_switching_pixel_tests_per_test(self):
>+        return False
>+
>+    def should_run_as_pixel_test(self, test_input):
>+        if not self._options.pixel_tests:
>+            return False
>+        if self._options.skip_pixel_test_if_no_baseline and not self.expected_image(test_input.test_name):
>+            return False
>+        if not self._options.pixel_test_directories:
>+            return True
>+        if any(test_input.test_name.startswith(directory) for directory in self._options.pixel_test_directories):
>+            return True
>+        return self._in_default_pixel_test_directory(test_input)
>+
>+    def _in_default_pixel_test_directory(self, test_input):
>+        # Ports can override this according to their default directories.
>+        return False
>+
> 
> class VirtualTestSuite(object):
>     def __init__(self, name, base, args, tests=None):
>diff --git a/Tools/Scripts/webkitpy/layout_tests/port/driver.py b/Tools/Scripts/webkitpy/layout_tests/port/driver.py
>index 5e7061bab976ca5df88d527d4a6b8d622e231558..a9cab89daac416da4c5c5306db6d3be56b1e0df3 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/port/driver.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/port/driver.py
>@@ -298,7 +298,7 @@ class Driver(object):
> 
>         cmd.extend(self._port.get_option('additional_drt_flag', []))
> 
>-        if pixel_tests:
>+        if pixel_tests and not self._port.supports_switching_pixel_tests_per_test():
>             cmd.append('--pixel-tests')
>         cmd.extend(per_test_args)
> 
>@@ -338,8 +338,11 @@ class Driver(object):
>             if sys.platform == 'cygwin':
>                 command = path.cygpath(command)
> 
>+        # "'" is the separator of command fields.
>+        if driver_input.should_run_pixel_test:
>+            if self._port.supports_switching_pixel_tests_per_test():
>+                command += "'" + '--pixel-test'
>         if driver_input.image_hash:
>-            # "'" is the separator of command fields.
>             command += "'" + driver_input.image_hash
>         return command + "\n"
> 
>diff --git a/Tools/Scripts/webkitpy/layout_tests/port/qt.py b/Tools/Scripts/webkitpy/layout_tests/port/qt.py
>index aaaafac5d364ccae47e9113e3855ad61f1a5c38d..7563a614167d42bd54455144420e11fcf6a3b745 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/port/qt.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/port/qt.py
>@@ -185,3 +185,14 @@ class QtPort(WebKitPort):
>             _log.error('Use git to grab the actual fonts from http://gitorious.org/qtwebkit/testfonts')
>             return False
>         return result
>+
>+    def _supports_switching_pixel_tests_per_test(self):
>+        return True
>+
>+    def _in_default_pixel_test_directory(self, test_input):
>+        return any(test_input.test_name.startswith(directory)
>+                   for directory in QtPort._default_pixel_test_directories())
>+
>+    @staticmethod
>+    def _default_pixel_test_directories():
>+        return ['compositing']
>diff --git a/Tools/Scripts/webkitpy/layout_tests/port/test.py b/Tools/Scripts/webkitpy/layout_tests/port/test.py
>index 9cf98de74885986ef1184365cc0ebfdf27861b1d..01d1954ee5878b644097627f97cbdab3ca8167f2 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/port/test.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/port/test.py
>@@ -227,6 +227,14 @@ layer at (0,0) size 800x34
>     tests.add('perf/foo/test.html')
>     tests.add('perf/foo/test-ref.html')
> 
>+    # For testing --pixel-test-directories.
>+    tests.add('failures/unexpected/pixeldir/image_in_pixeldir.html',
>+        actual_image='image_in_pixeldir-pngtEXtchecksum\x00checksum_fail',
>+        expected_image='image_in_pixeldir-pngtEXtchecksum\x00checksum-png')
>+    tests.add('failures/unexpected/image_not_in_pixeldir.html',
>+        actual_image='image_not_in_pixeldir-pngtEXtchecksum\x00checksum_fail',
>+        expected_image='image_not_in_pixeldir-pngtEXtchecksum\x00checksum-png')
>+
>     return tests
> 
> 
>diff --git a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
>index 455a8c2dc25b27e6de54e75e0108a0d1c2da0a53..88fceee047967003626eead4375a76e688cab467 100755
>--- a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
>@@ -185,6 +185,22 @@ def _set_up_derived_options(port, options):
>         options.reset_results = True
>         options.add_platform_exceptions = True
> 
>+    if options.pixel_test_directories:
>+        if not options.pixel_tests:
>+            warnings.append("--pixel-test-directories has no effect without --pixel-tests")
>+            options.pixel_test_directories = []
>+        else:
>+            sanitized = set()
>+            pixel_test_directories = options.pixel_test_directories
>+            for directory in pixel_test_directories:
>+                relative_path = port.make_relative_test_filename(directory)
>+                if not relative_path:
>+                    warnings.append("--pixel-test-directories expects list of directories but got: " + directory)
>+                else:
>+                    sanitized.add(relative_path)
>+
>+            options.pixel_test_directories = list(sanitized)
>+
>     return warnings
> 
> 
>@@ -198,6 +214,12 @@ def _compat_shim_option(option_name, **kwargs):
>         help="Ignored, for old-run-webkit-tests compat only.", **kwargs)
> 
> 
>+def _listarg_callback(option, opt_str, value, parser):
>+    values = parser.rargs[0].split(' ')
>+    setattr(parser.values, option.dest, values)
>+    del parser.rargs[0]
>+
>+
> def parse_args(args=None):
>     """Provides a default set of command line args.
> 
>@@ -315,6 +337,16 @@ def parse_args(args=None):
>         optparse.make_option("--skip-pixel-test-if-no-baseline", action="store_true",
>             dest="skip_pixel_test_if_no_baseline", help="Do not generate and check pixel result in the case when "
>                  "no image baseline is available for the test."),
>+        optparse.make_option("--pixel-test-directories", action="callback", default=[],
>+            dest="pixel_test_directories", callback=_listarg_callback,
>+            help="A string containing space separated list of directories where it is allowed to "
>+                 "execute tests as pixel tests. This option is supposed to be used togather with "
>+                 "--pixel-tests. If specified, only those tests will be executed as pixel tests "
>+                 "that are located in one of the directories enumerated here. Some ports may ignore "
>+                 "this option while others can have a default value that can be overridden here. "
>+                 '''
>+                 Example: \'animations compositing\'.'''),
>+
>         optparse.make_option("--skip-failing-tests", action="store_true",
>             default=False, help="Skip tests that are expected to fail. "
>                  "Note: When using this option, you might miss new crashes "
>diff --git a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py
>index ad14bf4efec47ba952c9913843fc3b90aa666402..ee1b60c4b1a0796f840915a0e54259a1d9073304 100755
>--- a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py
>@@ -179,7 +179,7 @@ def get_tests_run(extra_args=None, tests_included=False, flatten_batches=False,
> 
> # Update this magic number if you add an unexpected test to webkitpy.layout_tests.port.test
> # FIXME: It's nice to have a routine in port/test.py that returns this number.
>-unexpected_tests_count = 12
>+unexpected_tests_count = 14
> 
> 
> class StreamTestingMixin(object):
>@@ -501,7 +501,7 @@ class MainTest(unittest.TestCase, StreamTestingMixin):
> 
>     def test_run_singly_actually_runs_tests(self):
>         res, _, _, _ = logging_run(['--run-singly', 'failures/unexpected'])
>-        self.assertEquals(res, 8)
>+        self.assertEquals(res, 10)
> 
>     def test_single_file(self):
>         # FIXME: We should consider replacing more of the get_tests_run()-style tests
>@@ -573,6 +573,20 @@ class MainTest(unittest.TestCase, StreamTestingMixin):
>         self.assertTrue(json_string.find('"num_flaky":0') != -1)
>         self.assertTrue(json_string.find('"num_missing":1') != -1)
> 
>+    def test_pixel_test_directories(self):
>+        host = MockHost()
>+
>+        """Both tests have faling checksum. We include only the first in pixel tests so only that should fail."""
>+        args = ['--pixel-tests', '--pixel-test-directories', 'failures/unexpected/pixeldir',
>+                'failures/unexpected/pixeldir/image_in_pixeldir.html',
>+                'failures/unexpected/image_not_in_pixeldir.html']
>+        res, out, err, _ = logging_run(extra_args=args, host=host, record_results=True, tests_included=True)
>+
>+        self.assertEquals(res, 1)
>+        expected_token = '"unexpected":{"pixeldir":{"image_in_pixeldir.html":{"expected":"PASS","actual":"IMAGE"'
>+        json_string = host.filesystem.read_text_file('/tmp/layout-test-results/full_results.json')
>+        self.assertTrue(json_string.find(expected_token) != -1)
>+
>     def test_missing_and_unexpected_results_with_custom_exit_code(self):
>         # Test that we update expectations in place. If the expectation
>         # is missing, update the expected generic location.
>diff --git a/Tools/WebKitTestRunner/TestController.cpp b/Tools/WebKitTestRunner/TestController.cpp
>index 0a2cb2bdd7aadd22662251ce0d0e18c5580553ae..97fc61e2d03bcaeba573451cf5e7793f8703c937 100644
>--- a/Tools/WebKitTestRunner/TestController.cpp
>+++ b/Tools/WebKitTestRunner/TestController.cpp
>@@ -67,7 +67,7 @@ TestController& TestController::shared()
> }
> 
> TestController::TestController(int argc, const char* argv[])
>-    : m_dumpPixels(false)
>+    : m_dumpPixelsForAllTests(false)
>     , m_verbose(false)
>     , m_printSeparators(false)
>     , m_usingServerMode(false)
>@@ -261,7 +261,7 @@ void TestController::initialize(int argc, const char* argv[])
>         }
> 
>         if (argument == "--pixel-tests") {
>-            m_dumpPixels = true;
>+            m_dumpPixelsForAllTests = true;
>             continue;
>         }
>         if (argument == "--verbose") {
>@@ -510,18 +510,31 @@ bool TestController::runTest(const char* test)
>         return false;
>     }
> 
>-    std::string pathOrURL(test);
>+    bool dumpPixelsTest = m_dumpPixelsForAllTests;
>+    std::string command(test);
>+    std::string pathOrURL = command;
>     std::string expectedPixelHash;
>-    size_t separatorPos = pathOrURL.find("'");
>-    if (separatorPos != std::string::npos) {
>-        pathOrURL = std::string(std::string(test), 0, separatorPos);
>-        expectedPixelHash = std::string(std::string(test), separatorPos + 1);
>+    size_t firstSeparatorPos = command.find_first_of('\'');
>+    size_t secondSeparatorPos = command.find_first_of('\'', firstSeparatorPos + 1);
>+    if (firstSeparatorPos != std::string::npos) {
>+        pathOrURL = std::string(command, 0, firstSeparatorPos);
>+        size_t pixelHashPos = firstSeparatorPos + 1;
>+
>+        // NRWT passes --pixel-test if we should dump pixels for the test.
>+        const std::string expectedPixelTestArg("--pixel-test");
>+        std::string argTest = std::string(command, firstSeparatorPos + 1, expectedPixelTestArg.size());
>+        if (argTest == expectedPixelTestArg) {
>+            dumpPixelsTest = true;
>+            pixelHashPos = secondSeparatorPos == std::string::npos ? -1 : secondSeparatorPos + 1;
>+        }
>+        if (pixelHashPos != -1 && pixelHashPos < command.size())
>+            expectedPixelHash = std::string(command, pixelHashPos);
>     }
> 
>     m_state = RunningTest;
> 
>     m_currentInvocation = adoptPtr(new TestInvocation(pathOrURL));
>-    if (m_dumpPixels)
>+    if (dumpPixelsTest)
>         m_currentInvocation->setIsPixelTest(expectedPixelHash);
> 
>     m_currentInvocation->invoke();
>diff --git a/Tools/WebKitTestRunner/TestController.h b/Tools/WebKitTestRunner/TestController.h
>index b9465ef96e57e735c410447854193867f0b05cb0..6596a08c6bfcda8649e87627496b1bc10fd8e1a8 100644
>--- a/Tools/WebKitTestRunner/TestController.h
>+++ b/Tools/WebKitTestRunner/TestController.h
>@@ -104,7 +104,7 @@ private:
> 
>     OwnPtr<TestInvocation> m_currentInvocation;
> 
>-    bool m_dumpPixels;
>+    bool m_dumpPixelsForAllTests;
>     bool m_verbose;
>     bool m_printSeparators;
>     bool m_usingServerMode;

Actions: View | Formatted Diff | Diff

Attachments on bug 91754: 153526 | 153534 | 154015 | 154101 | 154412 | 154915