Diff
Modified: trunk/Tools/ChangeLog (123870 => 123871)
--- trunk/Tools/ChangeLog 2012-07-27 13:39:37 UTC (rev 123870)
+++ trunk/Tools/ChangeLog 2012-07-27 13:41:52 UTC (rev 123871)
@@ -1,3 +1,58 @@
+2012-07-27 Sheriff Bot <[email protected]>
+
+ Unreviewed, rolling out r123869.
+ http://trac.webkit.org/changeset/123869
+ https://bugs.webkit.org/show_bug.cgi?id=92501
+
+ "it did not fix anything but made things even worst"
+ (Requested by kbalazs on #webkit).
+
+ * 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):
+ * 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.supports_switching_pixel_tests_per_test):
+ (Port):
+ (Port._supports_switching_pixel_tests_per_test):
+ (Port.should_run_as_pixel_test):
+ (Port._should_run_as_pixel_test):
+ * Scripts/webkitpy/layout_tests/port/driver.py:
+ (Driver.cmd_line):
+ (Driver._command_from_driver_input):
+ * Scripts/webkitpy/layout_tests/port/qt.py:
+ (QtPort.check_sys_deps):
+ (QtPort):
+ (QtPort._supports_switching_pixel_tests_per_test):
+ (QtPort._should_run_as_pixel_test):
+ (QtPort._default_pixel_test_directories):
+ * Scripts/webkitpy/layout_tests/port/test.py:
+ (TestPort.supports_switching_pixel_tests_per_test):
+ * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+ (_set_up_derived_options):
+ (parse_args):
+ * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:
+ (MainTest.test_run_singly_actually_runs_tests):
+ (MainTest.test_pixel_test_directories):
+ * WebKitTestRunner/TestController.cpp:
+ (WTR::TestController::TestController):
+ (WTR::TestController::initialize):
+ (WTR::TestController::runTest):
+ * WebKitTestRunner/TestController.h:
+ (TestController):
+
2012-07-27 Balazs Kelemen <[email protected]>
[NRWT] should have a way to restrict pixel tests for individual directories
Modified: trunk/Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp (123870 => 123871)
--- trunk/Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp 2012-07-27 13:39:37 UTC (rev 123870)
+++ trunk/Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp 2012-07-27 13:41:52 UTC (rev 123871)
@@ -388,7 +388,7 @@
}
DumpRenderTree::DumpRenderTree()
- : m_dumpPixels(false)
+ : m_dumpPixelsForAllTests(false)
, m_stdin(0)
, m_enableTextOutput(false)
, m_standAloneMode(false)
@@ -606,7 +606,7 @@
if (isDumpAsTextTest(url)) {
layoutTestController()->dumpAsText();
- setDumpPixels(false);
+ setDumpPixelsForAllTests(false);
}
if (isGlobalHistoryTest(url))
@@ -692,14 +692,24 @@
{
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 @@
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 @@
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);
Modified: trunk/Tools/DumpRenderTree/qt/DumpRenderTreeQt.h (123870 => 123871)
--- trunk/Tools/DumpRenderTree/qt/DumpRenderTreeQt.h 2012-07-27 13:39:37 UTC (rev 123870)
+++ trunk/Tools/DumpRenderTree/qt/DumpRenderTreeQt.h 2012-07-27 13:41:52 UTC (rev 123871)
@@ -82,7 +82,7 @@
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 @@
QString dumpFrameScrollPosition(QWebFrame* frame);
LayoutTestController *m_controller;
- bool m_dumpPixels;
+ bool m_dumpPixelsForAllTests;
+ bool m_dumpPixelsForCurrentTest;
QString m_expectedHash;
QStringList m_standAloneModeTestList;
Modified: trunk/Tools/DumpRenderTree/qt/main.cpp (123870 => 123871)
--- trunk/Tools/DumpRenderTree/qt/main.cpp 2012-07-27 13:39:37 UTC (rev 123870)
+++ trunk/Tools/DumpRenderTree/qt/main.cpp 2012-07-27 13:41:52 UTC (rev 123871)
@@ -192,7 +192,7 @@
int index = args.indexOf(QLatin1String("--pixel-tests"));
if (index != -1) {
- dumper.setDumpPixels(true);
+ dumper.setDumpPixelsForAllTests(true);
args.removeAt(index);
}
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py (123870 => 123871)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py 2012-07-27 13:39:37 UTC (rev 123870)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py 2012-07-27 13:41:52 UTC (rev 123871)
@@ -78,10 +78,7 @@
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 @@
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 @@
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())
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py (123870 => 123871)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py 2012-07-27 13:39:37 UTC (rev 123870)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py 2012-07-27 13:41:52 UTC (rev 123871)
@@ -88,7 +88,8 @@
test_input.reference_files = self._port.reference_files(test_input.test_name)
if test_input.reference_files:
test_input.should_run_pixel_test = True
- test_input.should_run_pixel_test = self._options.pixel_tests
+ else:
+ 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)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py (123870 => 123871)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py 2012-07-27 13:39:37 UTC (rev 123870)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py 2012-07-27 13:41:52 UTC (rev 123871)
@@ -1256,7 +1256,31 @@
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):
+ # FIXME: all ports should support it.
+ return False
+
+ def should_run_as_pixel_test(self, test_input):
+ if not self._options.pixel_tests:
+ return False
+ if not self.supports_switching_pixel_tests_per_test():
+ # Cannot do more filtering without this.
+ return True
+ if self._options.pixel_test_directories:
+ return any(test_input.test_name.startswith(directory) for directory in self._options.pixel_test_directories)
+ return self._should_run_as_pixel_test(test_input)
+
+ def _should_run_as_pixel_test(self, test_input):
+ # Default behavior is to allow all test to run as pixel tests if --pixel-tests is on and
+ # --pixel-test-directory is not specified.
+ return True
+
+
class VirtualTestSuite(object):
def __init__(self, name, base, args, tests=None):
self.name = name
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/driver.py (123870 => 123871)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/driver.py 2012-07-27 13:39:37 UTC (rev 123870)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/driver.py 2012-07-27 13:41:52 UTC (rev 123871)
@@ -298,7 +298,7 @@
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,14 @@
if sys.platform == 'cygwin':
command = path.cygpath(command)
+ assert not driver_input.image_hash or driver_input.should_run_pixel_test
+
+ if driver_input.should_run_pixel_test:
+ if self._port.supports_switching_pixel_tests_per_test():
+ # We did not start the driver with --pixel-tests, instead we specify it per test.
+ # "'" is the separator of command fields.
+ command += "'" + '--pixel-test'
if driver_input.image_hash:
- # "'" is the separator of command fields.
command += "'" + driver_input.image_hash
return command + "\n"
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/qt.py (123870 => 123871)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/qt.py 2012-07-27 13:39:37 UTC (rev 123870)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/qt.py 2012-07-27 13:41:52 UTC (rev 123871)
@@ -185,3 +185,14 @@
_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 _should_run_as_pixel_test(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']
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py (123870 => 123871)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py 2012-07-27 13:39:37 UTC (rev 123870)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py 2012-07-27 13:41:52 UTC (rev 123871)
@@ -227,6 +227,14 @@
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
@@ -505,6 +513,11 @@
VirtualTestSuite('virtual/skipped', 'failures/expected', ['--virtual-arg2']),
]
+ def supports_switching_pixel_tests_per_test(self):
+ # Let it true so we can test the --pixel-test-directory option.
+ return True
+
+
class TestDriver(Driver):
"""Test/Dummy implementation of the DumpRenderTree interface."""
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py (123870 => 123871)
--- trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py 2012-07-27 13:39:37 UTC (rev 123870)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py 2012-07-27 13:41:52 UTC (rev 123871)
@@ -182,6 +182,22 @@
options.reset_results = True
options.add_platform_exceptions = True
+ if options.pixel_test_directories:
+ options.pixel_tests = True
+ varified_dirs = set()
+ pixel_test_directories = options.pixel_test_directories
+ for directory in pixel_test_directories:
+ # FIXME: we should support specifying the directories all the ways we support it for additional
+ # arguments specifying which tests and directories to run. We should also move the logic for that
+ # to Port.
+ filesystem = port.host.filesystem
+ if not filesystem.isdir(filesystem.join(port.layout_tests_dir(), directory)):
+ warnings.append("'%s' was passed to --pixel-test-directories, which doesn't seem to be a directory" % str(directory))
+ else:
+ varified_dirs.add(directory)
+
+ options.pixel_test_directories = list(varified_dirs)
+
return warnings
@@ -307,6 +323,15 @@
dest="new_test_results", default=True,
help="Don't create new baselines when no expected results exist"),
+ #FIXME: we should support a comma separated list with --pixel-test-directory as well.
+ optparse.make_option("--pixel-test-directory", action="" default=[], dest="pixel_test_directories",
+ help="A directory where it is allowed to execute tests as pixel tests. "
+ "Specify multiple times to add multiple directories. "
+ "This option implies --pixel-tests. If specified, only those tests "
+ "will be executed as pixel tests that are located in one of the "
+ "directories enumerated with the option. Some ports may ignore this "
+ "option while others can have a default value that can be overridden here."),
+
optparse.make_option("--skip-failing-tests", action=""
default=False, help="Skip tests that are expected to fail. "
"Note: When using this option, you might miss new crashes "
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py (123870 => 123871)
--- trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2012-07-27 13:39:37 UTC (rev 123870)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2012-07-27 13:41:52 UTC (rev 123871)
@@ -179,7 +179,7 @@
# 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):
@@ -496,7 +496,7 @@
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
@@ -568,6 +568,20 @@
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-directory', '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.
Modified: trunk/Tools/WebKitTestRunner/TestController.cpp (123870 => 123871)
--- trunk/Tools/WebKitTestRunner/TestController.cpp 2012-07-27 13:39:37 UTC (rev 123870)
+++ trunk/Tools/WebKitTestRunner/TestController.cpp 2012-07-27 13:41:52 UTC (rev 123871)
@@ -67,7 +67,7 @@
}
TestController::TestController(int argc, const char* argv[])
- : m_dumpPixels(false)
+ : m_dumpPixelsForAllTests(false)
, m_verbose(false)
, m_printSeparators(false)
, m_usingServerMode(false)
@@ -268,7 +268,7 @@
}
if (argument == "--pixel-tests") {
- m_dumpPixels = true;
+ m_dumpPixelsForAllTests = true;
continue;
}
if (argument == "--verbose") {
@@ -517,18 +517,31 @@
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 ? std::string::npos : secondSeparatorPos + 1;
+ }
+ if (pixelHashPos != std::string::npos && 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();
Modified: trunk/Tools/WebKitTestRunner/TestController.h (123870 => 123871)
--- trunk/Tools/WebKitTestRunner/TestController.h 2012-07-27 13:39:37 UTC (rev 123870)
+++ trunk/Tools/WebKitTestRunner/TestController.h 2012-07-27 13:41:52 UTC (rev 123871)
@@ -104,7 +104,7 @@
OwnPtr<TestInvocation> m_currentInvocation;
- bool m_dumpPixels;
+ bool m_dumpPixelsForAllTests;
bool m_verbose;
bool m_printSeparators;
bool m_usingServerMode;
