diff options
Diffstat (limited to 'Tools/Scripts/webkitpy')
55 files changed, 1179 insertions, 345 deletions
diff --git a/Tools/Scripts/webkitpy/common/checkout/api.py b/Tools/Scripts/webkitpy/common/checkout/api.py index 170b822..5c21028 100644 --- a/Tools/Scripts/webkitpy/common/checkout/api.py +++ b/Tools/Scripts/webkitpy/common/checkout/api.py @@ -35,7 +35,7 @@ from webkitpy.common.checkout.commitinfo import CommitInfo from webkitpy.common.checkout.scm import CommitMessage from webkitpy.common.checkout.deps import DEPS from webkitpy.common.memoized import memoized -from webkitpy.common.net.bugzilla import parse_bug_id +from webkitpy.common.net.bugzilla import parse_bug_id_from_changelog from webkitpy.common.system.executive import Executive, run_command, ScriptError from webkitpy.common.system.deprecated_logging import log @@ -85,7 +85,7 @@ class Checkout(object): return None changelog_entry = changelog_entries[0] changelog_data = { - "bug_id": parse_bug_id(changelog_entry.contents()), + "bug_id": parse_bug_id_from_changelog(changelog_entry.contents()), "author_name": changelog_entry.author_name(), "author_email": changelog_entry.author_email(), "author": changelog_entry.author(), @@ -145,7 +145,7 @@ class Checkout(object): def bug_id_for_this_commit(self, git_commit, changed_files=None): try: - return parse_bug_id(self.commit_message_for_this_commit(git_commit, changed_files).message()) + return parse_bug_id_from_changelog(self.commit_message_for_this_commit(git_commit, changed_files).message()) except ScriptError, e: pass # We might not have ChangeLogs. diff --git a/Tools/Scripts/webkitpy/common/checkout/changelog.py b/Tools/Scripts/webkitpy/common/checkout/changelog.py index ccdf9ca..a86b7a9 100644 --- a/Tools/Scripts/webkitpy/common/checkout/changelog.py +++ b/Tools/Scripts/webkitpy/common/checkout/changelog.py @@ -36,7 +36,7 @@ import textwrap from webkitpy.common.system.deprecated_logging import log from webkitpy.common.config.committers import CommitterList -from webkitpy.common.net.bugzilla import parse_bug_id +from webkitpy.common.net.bugzilla import parse_bug_id_from_changelog class ChangeLogEntry(object): @@ -87,7 +87,7 @@ class ChangeLogEntry(object): return self._contents def bug_id(self): - return parse_bug_id(self._contents) + return parse_bug_id_from_changelog(self._contents) # FIXME: Various methods on ChangeLog should move into ChangeLogEntry instead. diff --git a/Tools/Scripts/webkitpy/common/checkout/scm.py b/Tools/Scripts/webkitpy/common/checkout/scm.py index 70f65b5..e436402 100644 --- a/Tools/Scripts/webkitpy/common/checkout/scm.py +++ b/Tools/Scripts/webkitpy/common/checkout/scm.py @@ -29,6 +29,7 @@ # # Python module for interacting with an SCM system (like SVN or Git) +import logging import os import re import sys @@ -290,7 +291,7 @@ class SCM: def revert_files(self, file_paths): self._subclass_must_implement() - def commit_with_message(self, message, username=None, git_commit=None, force_squash=False): + def commit_with_message(self, message, username=None, git_commit=None, force_squash=False, changed_files=None): self._subclass_must_implement() def svn_commit_log(self, svn_revision): @@ -555,12 +556,8 @@ class SVN(SCM): # FIXME: This should probably use cwd=self.checkout_root. self.run(['svn', 'revert'] + file_paths) - def commit_with_message(self, message, username=None, git_commit=None, force_squash=False): + def commit_with_message(self, message, username=None, git_commit=None, force_squash=False, changed_files=None): # git-commit and force are not used by SVN. - if self.dryrun: - # Return a string which looks like a commit so that things which parse this output will succeed. - return "Dry run, no commit.\nCommitted revision 0." - svn_commit_args = ["svn", "commit"] if not username and not self.has_authorization_for_realm(): @@ -569,6 +566,17 @@ class SVN(SCM): svn_commit_args.extend(["--username", username]) svn_commit_args.extend(["-m", message]) + + if changed_files: + svn_commit_args.extend(changed_files) + + if self.dryrun: + _log = logging.getLogger("webkitpy.common.system") + _log.debug('Would run SVN command: "' + " ".join(svn_commit_args) + '"') + + # Return a string which looks like a commit so that things which parse this output will succeed. + return "Dry run, no commit.\nCommitted revision 0." + # FIXME: Should this use cwd=self.checkout_root? return self.run(svn_commit_args, error_handler=commit_error_handler) @@ -826,7 +834,7 @@ class Git(SCM): if num_local_commits > 1 or (num_local_commits > 0 and not working_directory_is_clean): raise AmbiguousCommitError(num_local_commits, working_directory_is_clean) - def commit_with_message(self, message, username=None, git_commit=None, force_squash=False): + def commit_with_message(self, message, username=None, git_commit=None, force_squash=False, changed_files=None): # Username is ignored during Git commits. working_directory_is_clean = self.working_directory_is_clean() diff --git a/Tools/Scripts/webkitpy/common/config/build.py b/Tools/Scripts/webkitpy/common/config/build.py index 355fa96..42d0721 100644 --- a/Tools/Scripts/webkitpy/common/config/build.py +++ b/Tools/Scripts/webkitpy/common/config/build.py @@ -33,17 +33,18 @@ def _should_file_trigger_build(target_platform, file): # precendence over later ones. # FIXME: The patterns below have only been verified to be correct on - # Windows. We should implement this for other platforms and start using - # it for their bots. Someone familiar with each platform will have to - # figure out what the right set of directories/patterns is for that - # platform. - assert(target_platform == "win") + # the platforms listed below. We should implement this for other platforms + # and start using it for their bots. Someone familiar with each platform + # will have to figure out what the right set of directories/patterns is for + # that platform. + assert(target_platform in ("mac-leopard", "mac-snowleopard", "win")) directories = [ # Directories that shouldn't trigger builds on any bots. + ("Examples", []), ("PerformanceTests", []), ("Source/WebCore/manual-tests", []), - ("Examples", []), + ("Tools/BuildSlaveSupport/build.webkit.org-config/public_html", []), ("Websites", []), ("android", []), ("brew", []), @@ -53,14 +54,13 @@ def _should_file_trigger_build(target_platform, file): ("opengl", []), ("opentype", []), ("openvg", []), - ("wx", []), ("wince", []), + ("wx", []), # Directories that should trigger builds on only some bots. ("Source/JavaScriptGlue", ["mac"]), - ("LayoutTests/platform/mac", ["mac", "win"]), - ("LayoutTests/platform/mac-snowleopard", ["mac-snowleopard", "win"]), ("Source/WebCore/image-decoders", ["chromium"]), + ("LayoutTests/platform/mac", ["mac", "win"]), ("cairo", ["gtk", "wincairo"]), ("cf", ["chromium-mac", "mac", "qt", "win"]), ("chromium", ["chromium"]), @@ -72,7 +72,7 @@ def _should_file_trigger_build(target_platform, file): ("gtk", ["gtk"]), ("mac", ["chromium-mac", "mac"]), ("mac-leopard", ["mac-leopard"]), - ("mac-snowleopard", ["mac-snowleopard"]), + ("mac-snowleopard", ["mac", "win"]), ("mac-wk2", ["mac-snowleopard", "win"]), ("objc", ["mac"]), ("qt", ["qt"]), diff --git a/Tools/Scripts/webkitpy/common/config/build_unittest.py b/Tools/Scripts/webkitpy/common/config/build_unittest.py index 1e075be..9144874 100644 --- a/Tools/Scripts/webkitpy/common/config/build_unittest.py +++ b/Tools/Scripts/webkitpy/common/config/build_unittest.py @@ -39,13 +39,13 @@ class ShouldBuildTest(unittest.TestCase): (["LayoutTests/platform/chromium-linux/foo"], ["chromium-linux"]), (["LayoutTests/platform/chromium-win/fast/compact/001-expected.txt"], ["chromium-win"]), (["LayoutTests/platform/mac-leopard/foo"], ["mac-leopard"]), - (["LayoutTests/platform/mac-snowleopard/foo"], ["mac-snowleopard", "win"]), + (["LayoutTests/platform/mac-snowleopard/foo"], ["mac-leopard", "mac-snowleopard", "win"]), (["LayoutTests/platform/mac-wk2/Skipped"], ["mac-snowleopard", "win"]), (["LayoutTests/platform/mac/foo"], ["mac-leopard", "mac-snowleopard", "win"]), (["LayoutTests/platform/win-xp/foo"], ["win"]), (["LayoutTests/platform/win-wk2/foo"], ["win"]), (["LayoutTests/platform/win/foo"], ["win"]), - (["Source/WebCore.exp.in", "Source/WebKit/mac/WebKit.exp"], ["mac"]), + (["Source/WebCore.exp.in", "Source/WebKit/mac/WebKit.exp"], ["mac-leopard", "mac-snowleopard"]), (["Source/WebCore/mac/foo"], ["chromium-mac", "mac-leopard", "mac-snowleopard"]), (["Source/WebCore/win/foo"], ["chromium-win", "win"]), (["Source/WebCore/platform/graphics/gpu/foo"], ["mac-leopard", "mac-snowleopard"]), @@ -53,13 +53,14 @@ class ShouldBuildTest(unittest.TestCase): (["Source/WebCore/rendering/RenderThemeMac.mm", "Source/WebCore/rendering/RenderThemeMac.h"], ["mac-leopard", "mac-snowleopard"]), (["Source/WebCore/rendering/RenderThemeChromiumLinux.h"], ["chromium-linux"]), (["Source/WebCore/rendering/RenderThemeWinCE.h"], []), + (["Tools/BuildSlaveSupport/build.webkit.org-config/public_html/LeaksViewer/LeaksViewer.js"], []), ] def test_should_build(self): for files, platforms in self._should_build_tests: # FIXME: We should test more platforms here once # build._should_file_trigger_build is implemented for them. - for platform in ["win"]: + for platform in ["mac-leopard", "mac-snowleopard", "win"]: should_build = platform in platforms or "*" in platforms self.assertEqual(build.should_build(platform, files), should_build, "%s should%s have built but did%s (files: %s)" % (platform, "" if should_build else "n't", "n't" if should_build else "", str(files))) diff --git a/Tools/Scripts/webkitpy/common/config/committers.py b/Tools/Scripts/webkitpy/common/config/committers.py index 76f4741..fd9bdbb 100644 --- a/Tools/Scripts/webkitpy/common/config/committers.py +++ b/Tools/Scripts/webkitpy/common/config/committers.py @@ -71,6 +71,7 @@ committers_unable_to_review = [ Committer("Alejandro G. Castro", ["alex@igalia.com", "alex@webkit.org"]), Committer("Alexander Kellett", ["lypanov@mac.com", "a-lists001@lypanov.net", "lypanov@kde.org"], "lypanov"), Committer("Alexander Pavlov", "apavlov@chromium.org", "apavlov"), + Committer("Alexis Menard", ["alexis.menard@openbossa.org", "menard@kde.org"], "darktears"), Committer("Andre Boule", "aboule@apple.com"), Committer("Andrei Popescu", "andreip@google.com", "andreip"), Committer("Andrew Wellington", ["andrew@webkit.org", "proton@wiretapped.net"], "proton"), @@ -87,7 +88,6 @@ committers_unable_to_review = [ Committer("Benjamin Otte", ["otte@gnome.org", "otte@webkit.org"], "otte"), Committer("Brent Fulgham", "bfulgham@webkit.org", "bfulgham"), Committer("Brett Wilson", "brettw@chromium.org", "brettx"), - Committer("Brian Weinstein", "bweinstein@apple.com", "bweinstein"), Committer("Cameron McCormack", "cam@webkit.org", "heycam"), Committer("Carlos Garcia Campos", ["cgarcia@igalia.com", "carlosgc@gnome.org", "carlosgc@webkit.org"], "KaL"), Committer("Carol Szabo", "carol.szabo@nokia.com"), @@ -119,7 +119,7 @@ committers_unable_to_review = [ Committer("Girish Ramakrishnan", ["girish@forwardbias.in", "ramakrishnan.girish@gmail.com"]), Committer("Graham Dennis", ["Graham.Dennis@gmail.com", "gdennis@webkit.org"]), Committer("Greg Bolsinga", "bolsinga@apple.com"), - Committer("Gyuyoung Kim", ["gyuyoung.kim@samsung.com", "gyuyoung@gmail.com", "gyuyoung@webkit.org"], "gyuyoung"), + Committer("Gyuyoung Kim", ["gyuyoung.kim@samsung.com", "gyuyoung.kim@webkit.org"], "gyuyoung"), Committer("Hans Wennborg", "hans@chromium.org", "hwennborg"), Committer("Hayato Ito", "hayato@chromium.org", "hayato"), Committer("Helder Correia", "helder@sencha.com", "helder"), @@ -142,7 +142,7 @@ committers_unable_to_review = [ Committer("Jochen Eisinger", "jochen@chromium.org", "jochen__"), Committer("John Abd-El-Malek", "jam@chromium.org", "jam"), Committer("John Gregg", ["johnnyg@google.com", "johnnyg@chromium.org"], "johnnyg"), - Committer("John Knottenbelt", ["jknotten@chromium.org"], "jknotten"), + Committer("John Knottenbelt", "jknotten@chromium.org", "jknotten"), Committer("Johnny Ding", ["jnd@chromium.org", "johnnyding.webkit@gmail.com"], "johnnyding"), Committer("Joone Hur", ["joone.hur@collabora.co.uk", "joone@kldp.org", "joone@webkit.org"], "joone"), Committer("Joost de Valk", ["joost@webkit.org", "webkit-dev@joostdevalk.nl"], "Altha"), @@ -152,6 +152,7 @@ committers_unable_to_review = [ Committer("Justin Schuh", "jschuh@chromium.org", "jschuh"), Committer("Keishi Hattori", "keishi@webkit.org", "keishi"), Committer("Kelly Norton", "knorton@google.com"), + Committer("Kenji Imasaki", "imasaki@chromium.org", "imasaki"), Committer("Kent Hansen", "kent.hansen@nokia.com", "khansen"), Committer("Kimmo Kinnunen", ["kimmo.t.kinnunen@nokia.com", "kimmok@iki.fi", "ktkinnun@webkit.org"], "kimmok"), Committer("Kinuko Yasuda", "kinuko@chromium.org", "kinuko"), @@ -160,6 +161,7 @@ committers_unable_to_review = [ Committer("Leandro Pereira", ["leandro@profusion.mobi", "leandro@webkit.org"], "acidx"), Committer("Levi Weintraub", ["leviw@chromium.org", "leviw@google.com", "lweintraub@apple.com"], "leviw"), Committer("Lucas De Marchi", ["lucas.demarchi@profusion.mobi", "demarchi@webkit.org"], "demarchi"), + Committer("Lucas Forschler", ["lforschler@apple.com"], "lforschler"), Committer("Luiz Agostini", ["luiz@webkit.org", "luiz.agostini@openbossa.org"], "lca"), Committer("Mads Ager", "ager@chromium.org"), Committer("Marcus Voltis Bulach", "bulach@chromium.org"), @@ -238,6 +240,7 @@ reviewers_list = [ Reviewer("Benjamin Poulain", ["benjamin@webkit.org", "benjamin.poulain@nokia.com", "ikipou@gmail.com"], "benjaminp"), Reviewer("Beth Dakin", "bdakin@apple.com", "dethbakin"), Reviewer("Brady Eidson", "beidson@apple.com", "bradee-oh"), + Reviewer("Brian Weinstein", "bweinstein@apple.com", "bweinstein"), Reviewer("Cameron Zwarich", ["zwarich@apple.com", "cwzwarich@apple.com", "cwzwarich@webkit.org"]), Reviewer("Chris Blumenberg", "cblu@apple.com", "cblu"), Reviewer("Chris Marrin", "cmarrin@apple.com", "cmarrin"), diff --git a/Tools/Scripts/webkitpy/common/config/ports.py b/Tools/Scripts/webkitpy/common/config/ports.py index 163d5ef..9a5a269 100644 --- a/Tools/Scripts/webkitpy/common/config/ports.py +++ b/Tools/Scripts/webkitpy/common/config/ports.py @@ -41,6 +41,10 @@ class WebKitPort(object): def script_path(cls, script_name): return os.path.join("Tools", "Scripts", script_name) + @classmethod + def script_shell_command(cls, script_name): + return [cls.script_path(script_name)] + @staticmethod def port(port_name): ports = { @@ -76,11 +80,11 @@ class WebKitPort(object): @classmethod def update_webkit_command(cls): - return [cls.script_path("update-webkit")] + return cls.script_shell_command("update-webkit") @classmethod def build_webkit_command(cls, build_style=None): - command = [cls.script_path("build-webkit")] + command = cls.script_shell_command("build-webkit") if build_style == "debug": command.append("--debug") if build_style == "release": @@ -89,19 +93,19 @@ class WebKitPort(object): @classmethod def run_javascriptcore_tests_command(cls): - return [cls.script_path("run-javascriptcore-tests")] + return cls.script_shell_command("run-javascriptcore-tests") @classmethod def run_webkit_tests_command(cls): - return [cls.script_path("run-webkit-tests")] + return cls.script_shell_command("run-webkit-tests") @classmethod def run_python_unittests_command(cls): - return [cls.script_path("test-webkitpy")] + return cls.script_shell_command("test-webkitpy") @classmethod def run_perl_unittests_command(cls): - return [cls.script_path("test-webkitperl")] + return cls.script_shell_command("test-webkitperl") @classmethod def layout_tests_results_path(cls): @@ -226,11 +230,10 @@ class ChromiumPort(WebKitPort): @classmethod def run_webkit_tests_command(cls): - return [ - cls.script_path("new-run-webkit-tests"), - "--chromium", - "--no-pixel-tests", - ] + command = cls.script_shell_command("new-run-webkit-tests") + command.append("--chromium") + command.append("--no-pixel-tests") + return command @classmethod def run_javascriptcore_tests_command(cls): diff --git a/Tools/Scripts/webkitpy/common/net/bugzilla/__init__.py b/Tools/Scripts/webkitpy/common/net/bugzilla/__init__.py index cfaf3b1..bde67c6 100644 --- a/Tools/Scripts/webkitpy/common/net/bugzilla/__init__.py +++ b/Tools/Scripts/webkitpy/common/net/bugzilla/__init__.py @@ -2,7 +2,7 @@ # We only export public API here. # FIXME: parse_bug_id should not be a free function. -from .bugzilla import Bugzilla, parse_bug_id +from .bugzilla import Bugzilla, parse_bug_id, parse_bug_id_from_changelog # Unclear if Bug and Attachment need to be public classes. from .bug import Bug from .attachment import Attachment diff --git a/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py b/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py index 17a8515..8daf92e 100644 --- a/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py +++ b/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py @@ -53,22 +53,34 @@ from webkitpy.thirdparty.BeautifulSoup import BeautifulSoup, SoupStrainer def parse_bug_id(message): if not message: return None - match = re.search("http\://webkit\.org/b/(?P<bug_id>\d+)", message) + match = re.search(Bugzilla.bug_url_short, message) if match: return int(match.group('bug_id')) - match = re.search( - Bugzilla.bug_server_regex + "show_bug\.cgi\?id=(?P<bug_id>\d+)", - message) + match = re.search(Bugzilla.bug_url_long, message) if match: return int(match.group('bug_id')) return None +# FIXME: parse_bug_id_from_changelog should not be a free function. +# Parse the bug ID out of a Changelog message based on the format that is +# used by prepare-ChangeLog +def parse_bug_id_from_changelog(message): + if not message: + return None + match = re.search("^\s*" + Bugzilla.bug_url_short + "$", message, re.MULTILINE) + if match: + return int(match.group('bug_id')) + match = re.search("^\s*" + Bugzilla.bug_url_long + "$", message, re.MULTILINE) + if match: + return int(match.group('bug_id')) + return None + def timestamp(): return datetime.now().strftime("%Y%m%d%H%M%S") -# A container for all of the logic for making and parsing buzilla queries. +# A container for all of the logic for making and parsing bugzilla queries. class BugzillaQueries(object): def __init__(self, bugzilla): @@ -210,6 +222,8 @@ class Bugzilla(object): bug_server_host = "bugs.webkit.org" bug_server_regex = "https?://%s/" % re.sub('\.', '\\.', bug_server_host) bug_server_url = "https://%s/" % bug_server_host + bug_url_long = bug_server_regex + r"show_bug\.cgi\?id=(?P<bug_id>\d+)(&ctype=xml)?" + bug_url_short = r"http\://webkit\.org/b/(?P<bug_id>\d+)" def quips(self): # We only fetch and parse the list of quips once per instantiation diff --git a/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py b/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py index 1d08ca5..2e75ca9 100644 --- a/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py +++ b/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py @@ -30,7 +30,7 @@ import unittest import datetime import StringIO -from .bugzilla import Bugzilla, BugzillaQueries, parse_bug_id +from .bugzilla import Bugzilla, BugzillaQueries, parse_bug_id, parse_bug_id_from_changelog from webkitpy.common.system.outputcapture import OutputCapture from webkitpy.tool.mocktool import MockBrowser @@ -192,6 +192,33 @@ ZEZpbmlzaExvYWRXaXRoUmVhc29uOnJlYXNvbl07Cit9CisKIEBlbmQKIAogI2VuZGlmCg== }], } + def test_parse_bug_id_from_changelog(self): + commit_text = ''' +2011-03-23 Ojan Vafai <ojan@chromium.org> + + Add failing result for WebKit2. All tests that require + focus fail on WebKit2. See https://bugs.webkit.org/show_bug.cgi?id=56988. + + * platform/mac-wk2/fast/css/pseudo-any-expected.txt: Added. + + ''' + + self.assertEquals(None, parse_bug_id_from_changelog(commit_text)) + + commit_text = ''' +2011-03-23 Ojan Vafai <ojan@chromium.org> + + Add failing result for WebKit2. All tests that require + focus fail on WebKit2. See https://bugs.webkit.org/show_bug.cgi?id=56988. + https://bugs.webkit.org/show_bug.cgi?id=12345 + + * platform/mac-wk2/fast/css/pseudo-any-expected.txt: Added. + + ''' + + self.assertEquals(12345, parse_bug_id_from_changelog(commit_text)) + + # FIXME: This should move to a central location and be shared by more unit tests. def _assert_dictionaries_equal(self, actual, expected): # Make sure we aren't parsing more or less than we expect diff --git a/Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py b/Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py index 9dd165c..d23a6cc 100644 --- a/Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py +++ b/Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py @@ -42,9 +42,11 @@ import urllib2 from webkitpy.common.net.failuremap import FailureMap from webkitpy.common.net.layouttestresults import LayoutTestResults from webkitpy.common.net.regressionwindow import RegressionWindow +from webkitpy.common.net.testoutputset import TestOutputSet from webkitpy.common.system.logutils import get_logger -from webkitpy.thirdparty.autoinstalled.mechanize import Browser +from webkitpy.common.system.zipfileset import ZipFileSet from webkitpy.thirdparty.BeautifulSoup import BeautifulSoup +from webkitpy.thirdparty.autoinstalled.mechanize import Browser _log = get_logger(__file__) @@ -92,6 +94,12 @@ class Builder(object): self._builds_cache[build_number] = build return build + def latest_cached_build(self): + revision_build_pairs = self.revision_build_pairs_with_results() + revision_build_pairs.sort(key=lambda i: i[1]) + latest_build_number = revision_build_pairs[-1][1] + return self.build(latest_build_number) + def force_build(self, username="webkit-patch", comments=None): def predicate(form): try: @@ -221,6 +229,12 @@ class Build(object): results_directory = "r%s (%s)" % (self.revision(), self._number) return "%s/%s" % (self._builder.results_url(), urllib.quote(results_directory)) + def results_zip_url(self): + return "%s.zip" % self.results_url() + + def results(self): + return TestOutputSet(self._builder.name(), None, ZipFileSet(self.results_zip_url()), include_expected=False) + def _fetch_results_html(self): results_html = "%s/results.html" % (self.results_url()) # FIXME: This should use NetworkTransaction's 404 handling instead. @@ -268,8 +282,10 @@ class BuildBot(object): "SnowLeopard.*Build", "SnowLeopard.*\(Test", "SnowLeopard.*\(WebKit2 Test", - "Leopard.*Release", + "Leopard.*", "Windows.*Build", + "Windows.*\(Test", + "WinCairo", "WinCE", "EFL", "GTK.*32", diff --git a/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py b/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py index 6addb56..300bc88 100644 --- a/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py +++ b/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py @@ -231,32 +231,37 @@ class BuildBotTest(unittest.TestCase): {'name': u'SnowLeopard Intel Release (WebKit2 Tests)', }, {'name': u'SnowLeopard Intel Leaks', }, {'name': u'Windows Release (Build)', }, - {'name': u'Windows Release (Tests)', }, + {'name': u'Windows 7 Release (Tests)', }, {'name': u'Windows Debug (Build)', }, - {'name': u'Windows Debug (Tests)', }, + {'name': u'Windows XP Debug (Tests)', }, + {'name': u'Windows 7 Release (WebKit2 Tests)', }, {'name': u'GTK Linux 32-bit Release', }, {'name': u'GTK Linux 32-bit Debug', }, {'name': u'GTK Linux 64-bit Debug', }, - {'name': u'GTK Linux 64-bit Release', }, {'name': u'Qt Linux Release', }, {'name': u'Qt Linux Release minimal', }, {'name': u'Qt Linux ARMv7 Release', }, {'name': u'Qt Windows 32-bit Release', }, {'name': u'Qt Windows 32-bit Debug', }, - {'name': u'Chromium Linux Release', }, - {'name': u'Chromium Mac Release', }, {'name': u'Chromium Win Release', }, - {'name': u'Chromium Linux Release (Tests)', }, - {'name': u'Chromium Mac Release (Tests)', }, + {'name': u'Chromium Mac Release', }, + {'name': u'Chromium Linux Release', }, {'name': u'Chromium Win Release (Tests)', }, + {'name': u'Chromium Mac Release (Tests)', }, + {'name': u'Chromium Linux Release (Tests)', }, {'name': u'New run-webkit-tests', }, + {'name': u'WinCairo Debug (Build)', }, + {'name': u'WinCE Release (Build)', }, + {'name': u'EFL Linux Release (Build)', }, ] name_regexps = [ "SnowLeopard.*Build", "SnowLeopard.*\(Test", "SnowLeopard.*\(WebKit2 Test", - "Leopard.*Release", + "Leopard.*", "Windows.*Build", + "Windows.*\(Test", + "WinCairo", "WinCE", "EFL", "GTK.*32", @@ -267,11 +272,15 @@ class BuildBotTest(unittest.TestCase): expected_builders = [ {'name': u'Leopard Intel Release (Build)', }, {'name': u'Leopard Intel Release (Tests)', }, + {'name': u'Leopard Intel Debug (Build)', }, + {'name': u'Leopard Intel Debug (Tests)', }, {'name': u'SnowLeopard Intel Release (Build)', }, {'name': u'SnowLeopard Intel Release (Tests)', }, {'name': u'SnowLeopard Intel Release (WebKit2 Tests)', }, {'name': u'Windows Release (Build)', }, + {'name': u'Windows 7 Release (Tests)', }, {'name': u'Windows Debug (Build)', }, + {'name': u'Windows XP Debug (Tests)', }, {'name': u'GTK Linux 32-bit Release', }, {'name': u'GTK Linux 32-bit Debug', }, {'name': u'GTK Linux 64-bit Debug', }, @@ -280,9 +289,12 @@ class BuildBotTest(unittest.TestCase): {'name': u'Qt Linux ARMv7 Release', }, {'name': u'Qt Windows 32-bit Release', }, {'name': u'Qt Windows 32-bit Debug', }, - {'name': u'Chromium Linux Release', }, - {'name': u'Chromium Mac Release', }, {'name': u'Chromium Win Release', }, + {'name': u'Chromium Mac Release', }, + {'name': u'Chromium Linux Release', }, + {'name': u'WinCairo Debug (Build)', }, + {'name': u'WinCE Release (Build)', }, + {'name': u'EFL Linux Release (Build)', }, ] # This test should probably be updated if the default regexp list changes @@ -410,6 +422,33 @@ class BuildBotTest(unittest.TestCase): buildbot._latest_builds_from_builders = mock_builds_from_builders self.assertEqual(buildbot.last_green_revision(), 1) + def _fetch_build(self, build_number): + if build_number == 5: + return "correct build" + return "wrong build" + + def _fetch_revision_to_build_map(self): + return {'r5': 5, 'r2': 2, 'r3': 3} + + def test_latest_cached_build(self): + b = Builder('builder', BuildBot()) + b._fetch_build = self._fetch_build + b._fetch_revision_to_build_map = self._fetch_revision_to_build_map + self.assertEquals("correct build", b.latest_cached_build()) + + def results_url(self): + return "some-url" + + def test_results_zip_url(self): + b = Build(None, 123, 123, False) + b.results_url = self.results_url + self.assertEquals("some-url.zip", b.results_zip_url()) + + def test_results(self): + builder = Builder('builder', BuildBot()) + b = Build(builder, 123, 123, True) + self.assertTrue(b.results()) + if __name__ == '__main__': unittest.main() diff --git a/Tools/Scripts/webkitpy/common/system/autoinstall.py b/Tools/Scripts/webkitpy/common/system/autoinstall.py index 9adab29..4ffcccc 100755 --- a/Tools/Scripts/webkitpy/common/system/autoinstall.py +++ b/Tools/Scripts/webkitpy/common/system/autoinstall.py @@ -61,7 +61,7 @@ class AutoInstaller(object): installer.install(url="http://pypi.python.org/packages/source/p/pep8/pep8-0.5.0.tar.gz#md5=512a818af9979290cd619cce8e9c2e2b", url_subpath="pep8-0.5.0/pep8.py") - installer.install(url="http://pypi.python.org/packages/source/m/mechanize/mechanize-0.1.11.zip", + installer.install(url="http://pypi.python.org/packages/source/m/mechanize/mechanize-0.2.4.zip", url_subpath="mechanize") """ @@ -512,6 +512,6 @@ if __name__=="__main__": url_subpath="pep8-0.5.0/pep8.py") installer.install(should_refresh=False, target_name="mechanize", - url="http://pypi.python.org/packages/source/m/mechanize/mechanize-0.1.11.zip", + url="http://pypi.python.org/packages/source/m/mechanize/mechanize-0.2.4.zip", url_subpath="mechanize") diff --git a/Tools/Scripts/webkitpy/common/system/filesystem.py b/Tools/Scripts/webkitpy/common/system/filesystem.py index b876807..1988546 100644 --- a/Tools/Scripts/webkitpy/common/system/filesystem.py +++ b/Tools/Scripts/webkitpy/common/system/filesystem.py @@ -195,6 +195,9 @@ class FileSystem(object): mode = 'a' return codecs.open(path, mode, 'utf8') + def open_binary_file_for_reading(self, path): + return codecs.open(path, 'rb') + def read_binary_file(self, path): """Return the contents of the file at the given path as a byte string.""" with file(path, 'rb') as f: diff --git a/Tools/Scripts/webkitpy/common/system/filesystem_mock.py b/Tools/Scripts/webkitpy/common/system/filesystem_mock.py index aa79a8c..a6d158a 100644 --- a/Tools/Scripts/webkitpy/common/system/filesystem_mock.py +++ b/Tools/Scripts/webkitpy/common/system/filesystem_mock.py @@ -228,6 +228,11 @@ class MockFileSystem(object): def read_text_file(self, path): return self.read_binary_file(path).decode('utf-8') + def open_binary_file_for_reading(self, path): + if self.files[path] is None: + self._raise_not_found(path) + return ReadableFileObject(self, path, self.files[path]) + def read_binary_file(self, path): # Intentionally raises KeyError if we don't recognize the path. if self.files[path] is None: @@ -285,3 +290,28 @@ class WritableFileObject(object): def write(self, str): self.fs.files[self.path] += str self.fs.written_files[self.path] = self.fs.files[self.path] + + +class ReadableFileObject(object): + def __init__(self, fs, path, data=""): + self.fs = fs + self.path = path + self.closed = False + self.data = data + self.offset = 0 + + def __enter__(self): + return self + + def __exit__(self, type, value, traceback): + self.close() + + def close(self): + self.closed = True + + def read(self, bytes=None): + if not bytes: + return self.data[self.offset:] + start = self.offset + self.offset += bytes + return self.data[start:self.offset] diff --git a/Tools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py b/Tools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py index 83b2215..6d5cda8 100644 --- a/Tools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py +++ b/Tools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py @@ -211,7 +211,7 @@ class TestShellThread(threading.Thread, worker_mixin.WorkerMixin): self._num_tests_in_current_group = len(self._filename_list) self._current_group_start_time = time.time() - test_input = self._filename_list.pop() + test_input = self._filename_list.pop(0) # We have a url, run tests. self._num_tests += 1 diff --git a/Tools/Scripts/webkitpy/layout_tests/layout_package/single_test_runner.py b/Tools/Scripts/webkitpy/layout_tests/layout_package/single_test_runner.py index d755f67..a8c716f 100644 --- a/Tools/Scripts/webkitpy/layout_tests/layout_package/single_test_runner.py +++ b/Tools/Scripts/webkitpy/layout_tests/layout_package/single_test_runner.py @@ -44,14 +44,6 @@ def run_single_test(port, options, test_input, driver, worker_name): return runner.run() -class ExpectedDriverOutput: - """Groups information about an expected driver output.""" - def __init__(self, text, image, image_hash): - self.text = text - self.image = image - self.image_hash = image_hash - - class SingleTestRunner: def __init__(self, options, port, driver, test_input, worker_name): @@ -63,10 +55,43 @@ class SingleTestRunner: self._worker_name = worker_name self._testname = port.relative_test_filename(test_input.filename) + self._is_reftest = False + self._is_mismatch_reftest = False + self._reference_filename = None + + fs = port._filesystem + reftest_expected_filename = port.reftest_expected_filename(self._filename) + if fs.exists(reftest_expected_filename): + self._is_reftest = True + self._reference_filename = reftest_expected_filename + + reftest_expected_mismatch_filename = port.reftest_expected_mismatch_filename(self._filename) + if fs.exists(reftest_expected_mismatch_filename): + if self._is_reftest: + _log.error('It is not allowed that one test file has both' + ' expected.html file and expected-mismatch.html file' + ' at the same time. Please remove either %s or %s.', + reftest_expected_filename, reftest_expected_mismatch_filename) + else: + self._is_reftest = True + self._is_mismatch_reftest = True + self._reference_filename = reftest_expected_mismatch_filename + + if self._is_reftest: + # Detect and report a test which has a wrong combination of expectation files. + # For example, if 'foo.html' has two expectation files, 'foo-expected.html' and + # 'foo-expected.txt', we should warn users. One test file must be used exclusively + # in either layout tests or reftests, but not in both. + for suffix in ['.txt', '.checksum', '.png']: + expected_filename = self._port.expected_filename(self._filename, suffix) + if fs.exists(expected_filename): + _log.error('The reftest (%s) can not have an expectation file (%s).' + ' Please remove that file.', self._testname, expected_filename) + def _expected_driver_output(self): - return ExpectedDriverOutput(self._port.expected_text(self._filename), - self._port.expected_image(self._filename), - self._port.expected_checksum(self._filename)) + return base.DriverOutput(self._port.expected_text(self._filename), + self._port.expected_image(self._filename), + self._port.expected_checksum(self._filename)) def _should_fetch_expected_checksum(self): return (self._options.pixel_tests and @@ -84,7 +109,13 @@ class SingleTestRunner: def run(self): if self._options.new_baseline or self._options.reset_results: - return self._run_rebaseline() + if self._is_reftest: + # Returns a dummy TestResult. We don't have to rebase for reftests. + return TestResult(self._filename) + else: + return self._run_rebaseline() + if self._is_reftest: + return self._run_reftest() return self._run_compare_test() def _run_compare_test(self): @@ -98,6 +129,8 @@ class SingleTestRunner: def _run_rebaseline(self): driver_output = self._driver.run_test(self._driver_input()) failures = self._handle_error(driver_output) + test_result_writer.write_test_result(self._port, self._options.results_directory, self._filename, + driver_output, None, failures) # FIXME: It the test crashed or timed out, it might be bettter to avoid # to write new baselines. self._save_baselines(driver_output) @@ -145,22 +178,31 @@ class SingleTestRunner: port.update_baseline(output_path, data) - def _handle_error(self, driver_output): + def _handle_error(self, driver_output, reference_filename=None): + """Returns test failures if some unusual errors happen in driver's run. + + Args: + driver_output: The output from the driver. + reference_filename: The full path to the reference file which produced the driver_output. + This arg is optional and should be used only in reftests until we have a better way to know + which html file is used for producing the driver_output. + """ failures = [] fs = self._port._filesystem if driver_output.timeout: - failures.append(test_failures.FailureTimeout()) + failures.append(test_failures.FailureTimeout(reference_filename)) + + if reference_filename: + testname = self._port.relative_test_filename(reference_filename) + else: + testname = self._testname + if driver_output.crash: - failures.append(test_failures.FailureCrash()) - _log.debug("%s Stacktrace for %s:\n%s" % (self._worker_name, self._testname, + failures.append(test_failures.FailureCrash(reference_filename)) + _log.debug("%s Stacktrace for %s:\n%s" % (self._worker_name, testname, driver_output.error)) - # FIXME: Use test_result_writer module. - stack_filename = fs.join(self._options.results_directory, self._testname) - stack_filename = fs.splitext(stack_filename)[0] + "-stack.txt" - fs.maybe_make_directory(fs.dirname(stack_filename)) - fs.write_text_file(stack_filename, driver_output.error) elif driver_output.error: - _log.debug("%s %s output stderr lines:\n%s" % (self._worker_name, self._testname, + _log.debug("%s %s output stderr lines:\n%s" % (self._worker_name, testname, driver_output.error)) return failures @@ -210,3 +252,31 @@ class SingleTestRunner: elif driver_output.image_hash != expected_driver_outputs.image_hash: failures.append(test_failures.FailureImageHashMismatch()) return failures + + def _run_reftest(self): + driver_output1 = self._driver.run_test(self._driver_input()) + driver_output2 = self._driver.run_test( + base.DriverInput(self._reference_filename, self._timeout, driver_output1.image_hash)) + test_result = self._compare_output_with_reference(driver_output1, driver_output2) + + test_result_writer.write_test_result(self._port, self._options.results_directory, self._filename, + driver_output1, driver_output2, test_result.failures) + return test_result + + def _compare_output_with_reference(self, driver_output1, driver_output2): + total_test_time = driver_output1.test_time + driver_output2.test_time + failures = [] + failures.extend(self._handle_error(driver_output1)) + if failures: + # Don't continue any more if we already have crash or timeout. + return TestResult(self._filename, failures, total_test_time) + failures.extend(self._handle_error(driver_output2, reference_filename=self._reference_filename)) + if failures: + return TestResult(self._filename, failures, total_test_time) + + if self._is_mismatch_reftest: + if driver_output1.image_hash == driver_output2.image_hash: + failures.append(test_failures.FailureReftestMismatchDidNotOccur()) + elif driver_output1.image_hash != driver_output2.image_hash: + failures.append(test_failures.FailureReftestMismatch()) + return TestResult(self._filename, failures, total_test_time) diff --git a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py index eb59d36..1fad772 100644 --- a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py +++ b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py @@ -60,11 +60,13 @@ def determine_result_type(failure_list): is_text_failure = FailureTextMismatch in failure_types is_image_failure = (FailureImageHashIncorrect in failure_types or FailureImageHashMismatch in failure_types) + is_reftest_failure = (FailureReftestMismatch in failure_types or + FailureReftestMismatchDidNotOccur in failure_types) if is_text_failure and is_image_failure: return test_expectations.IMAGE_PLUS_TEXT elif is_text_failure: return test_expectations.TEXT - elif is_image_failure: + elif is_image_failure or is_reftest_failure: return test_expectations.IMAGE else: raise ValueError("unclassifiable set of failures: " @@ -126,8 +128,8 @@ class TestFailure(object): return filename[:filename.rfind('.')] + modifier -class FailureWithType(TestFailure): - """Base class that produces standard HTML output based on the test type. +class ComparisonTestFailure(TestFailure): + """Base class that produces standard HTML output based on the result of the comparison test. Subclasses may commonly choose to override the ResultHtmlOutput, but still use the standard OutputLinks. @@ -177,11 +179,17 @@ class FailureTimeout(TestFailure): """Test timed out. We also want to restart DumpRenderTree if this happens.""" + def __init__(self, reference_filename=None): + self.reference_filename = reference_filename + @staticmethod def message(): return "Test timed out" def result_html_output(self, filename): + if self.reference_filename: + return "<strong>%s</strong> (occured in <a href=%s>expected html</a>)" % ( + self.message(), self.reference_filename) return "<strong>%s</strong>" % self.message() def should_kill_dump_render_tree(self): @@ -191,6 +199,9 @@ class FailureTimeout(TestFailure): class FailureCrash(TestFailure): """DumpRenderTree crashed.""" + def __init__(self, reference_filename=None): + self.reference_filename = reference_filename + @staticmethod def message(): return "DumpRenderTree crashed" @@ -198,14 +209,17 @@ class FailureCrash(TestFailure): def result_html_output(self, filename): # FIXME: create a link to the minidump file stack = self.relative_output_filename(filename, "-stack.txt") - return "<strong>%s</strong> <a href=%s>stack</a>" % (self.message(), - stack) + if self.reference_filename: + return "<strong>%s</strong> <a href=%s>stack</a> (occured in <a href=%s>expected html</a>)" % ( + self.message(), stack, self.reference_filename) + else: + return "<strong>%s</strong> <a href=%s>stack</a>" % (self.message(), stack) def should_kill_dump_render_tree(self): return True -class FailureMissingResult(FailureWithType): +class FailureMissingResult(ComparisonTestFailure): """Expected result was missing.""" OUT_FILENAMES = ("-actual.txt",) @@ -218,7 +232,7 @@ class FailureMissingResult(FailureWithType): self.output_links(filename, self.OUT_FILENAMES)) -class FailureTextMismatch(FailureWithType): +class FailureTextMismatch(ComparisonTestFailure): """Text diff output failed.""" # Filename suffixes used by ResultHtmlOutput. # FIXME: Why don't we use the constants from TestTypeBase here? @@ -230,7 +244,7 @@ class FailureTextMismatch(FailureWithType): return "Text diff mismatch" -class FailureMissingImageHash(FailureWithType): +class FailureMissingImageHash(ComparisonTestFailure): """Actual result hash was missing.""" # Chrome doesn't know to display a .checksum file as text, so don't bother # putting in a link to the actual result. @@ -243,7 +257,7 @@ class FailureMissingImageHash(FailureWithType): return "<strong>%s</strong>" % self.message() -class FailureMissingImage(FailureWithType): +class FailureMissingImage(ComparisonTestFailure): """Actual result image was missing.""" OUT_FILENAMES = ("-actual.png",) @@ -256,7 +270,7 @@ class FailureMissingImage(FailureWithType): self.output_links(filename, self.OUT_FILENAMES)) -class FailureImageHashMismatch(FailureWithType): +class FailureImageHashMismatch(ComparisonTestFailure): """Image hashes didn't match.""" OUT_FILENAMES = ("-actual.png", "-expected.png", "-diff.png") @@ -267,7 +281,7 @@ class FailureImageHashMismatch(FailureWithType): return "Image mismatch" -class FailureImageHashIncorrect(FailureWithType): +class FailureImageHashIncorrect(ComparisonTestFailure): """Actual result hash is incorrect.""" # Chrome doesn't know to display a .checksum file as text, so don't bother # putting in a link to the actual result. @@ -279,9 +293,48 @@ class FailureImageHashIncorrect(FailureWithType): def result_html_output(self, filename): return "<strong>%s</strong>" % self.message() + +class FailureReftestMismatch(ComparisonTestFailure): + """The result didn't match the reference rendering.""" + + OUT_FILENAMES = ("-expected.html", "-expected.png", "-actual.png", + "-diff.png",) + + @staticmethod + def message(): + return "Mismatch with reference" + + def output_links(self, filename, out_names): + links = [''] + uris = [self.relative_output_filename(filename, output_filename) + for output_filename in out_names] + for text, uri in zip(['-expected.html', 'expected', 'actual', 'diff'], uris): + links.append("<a href='%s'>%s</a>" % (uri, text)) + return ' '.join(links) + + +class FailureReftestMismatchDidNotOccur(ComparisonTestFailure): + """Unexpected match between the result and the reference rendering.""" + + OUT_FILENAMES = ("-expected-mismatch.html", "-actual.png",) + + @staticmethod + def message(): + return "Mismatch with the reference did not occur" + + def output_links(self, filename, out_names): + links = [''] + uris = [self.relative_output_filename(filename, output_filename) + for output_filename in out_names] + for text, uri in zip(['-expected-mismatch.html', 'image'], uris): + links.append("<a href='%s'>%s</a>" % (uri, text)) + return ' '.join(links) + + # Convenient collection of all failure classes for anything that might # need to enumerate over them all. ALL_FAILURE_CLASSES = (FailureTimeout, FailureCrash, FailureMissingResult, FailureTextMismatch, FailureMissingImageHash, FailureMissingImage, FailureImageHashMismatch, - FailureImageHashIncorrect) + FailureImageHashIncorrect, FailureReftestMismatch, + FailureReftestMismatchDidNotOccur) diff --git a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py index 882da91..e209503 100644 --- a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py +++ b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py @@ -64,7 +64,17 @@ def write_test_result(port, root_output_dir, filename, driver_output, checksums_mismatch_but_images_are_same = True imagehash_mismatch_failure = failure elif isinstance(failure, test_failures.FailureCrash): - writer.write_crash_report(driver_output.error) + if failure.reference_filename: + writer.write_crash_report(expected_driver_output.error) + else: + writer.write_crash_report(driver_output.error) + elif isinstance(failure, test_failures.FailureReftestMismatch): + writer.write_image_files(driver_output.image, expected_driver_output.image) + writer.create_image_diff_and_write_result(driver_output.image, expected_driver_output.image) + writer.copy_file(port.reftest_expected_filename(filename), '-expected.html') + elif isinstance(failure, test_failures.FailureReftestMismatchDidNotOccur): + writer.write_image_files(driver_output.image, expected_image=None) + writer.copy_file(port.reftest_expected_mismatch_filename(filename), '-expected-mismatch.html') else: assert isinstance(failure, (test_failures.FailureTimeout,)) @@ -135,9 +145,9 @@ class TestResultWriter(object): expected_filename = self.output_filename(self.FILENAME_SUFFIX_EXPECTED + file_type) fs = self._port._filesystem - if output: + if output is not None: fs.write_binary_file(actual_filename, output) - if expected: + if expected is not None: fs.write_binary_file(expected_filename, expected) def write_crash_report(self, error): @@ -193,3 +203,10 @@ class TestResultWriter(object): # To do so, we have to change port.diff_image() as well. diff_filename = self.output_filename(self.FILENAME_SUFFIX_IMAGE_DIFF) return self._port.diff_image(actual_image, expected_image, diff_filename) + + def copy_file(self, src_filepath, dst_extension): + fs = self._port._filesystem + assert fs.exists(src_filepath), 'src_filepath: %s' % src_filepath + dst_filename = self.output_filename(dst_extension) + self._make_output_directory() + fs.copyfile(src_filepath, dst_filename) diff --git a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py index 0859f68..569dd51 100644 --- a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py +++ b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py @@ -495,10 +495,6 @@ class TestRunner: # a PriorityQueue until we move to Python 2.6. for directory in tests_by_dir: test_list = tests_by_dir[directory] - # Keep the tests in alphabetical order. - # FIXME: Remove once tests are fixed so they can be run in any - # order. - test_list.reverse() test_list_tuple = (directory, test_list) test_lists.append(test_list_tuple) test_lists.sort(lambda a, b: cmp(len(b[1]), len(a[1]))) @@ -507,7 +503,6 @@ class TestRunner: # but each http test takes a very long time to run, so sorting by the # number of tests doesn't accurately capture how long they take to run. if tests_to_http_lock: - tests_to_http_lock.reverse() test_lists.insert(0, ("tests_to_http_lock", tests_to_http_lock)) return test_lists diff --git a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner2.py b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner2.py index 0522d39..5a6344c 100644 --- a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner2.py +++ b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner2.py @@ -224,7 +224,7 @@ class TestRunner2(test_runner.TestRunner): if worker_state.wedged: # This shouldn't happen if we have our timeouts tuned properly. - _log.error("%s unwedged", w.name) + _log.error("%s unwedged", source) self._all_results.append(result) self._update_summary_with_result(self._current_result_summary, result) diff --git a/Tools/Scripts/webkitpy/layout_tests/port/base.py b/Tools/Scripts/webkitpy/layout_tests/port/base.py index 247a260..dea126f 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/base.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/base.py @@ -30,6 +30,8 @@ """Abstract base class of Port-specific entrypoints for the layout tests test infrastructure (the Port and Driver classes).""" +from __future__ import with_statement + import cgi import difflib import errno @@ -44,20 +46,19 @@ try: except ImportError: multiprocessing = None -import apache_http_server -import config as port_config -import http_lock -import http_server -import test_files -import websocket_server - from webkitpy.common import system from webkitpy.common.system import filesystem from webkitpy.common.system import logutils from webkitpy.common.system import path from webkitpy.common.system.executive import Executive, ScriptError from webkitpy.common.system.user import User - +from webkitpy.layout_tests import read_checksum_from_png +from webkitpy.layout_tests.port import apache_http_server +from webkitpy.layout_tests.port import config as port_config +from webkitpy.layout_tests.port import http_lock +from webkitpy.layout_tests.port import http_server +from webkitpy.layout_tests.port import test_files +from webkitpy.layout_tests.port import websocket_server _log = logutils.get_logger(__file__) @@ -139,7 +140,9 @@ class Port(object): return self._executive.cpu_count() def default_worker_model(self): - return 'old-threads' + if self._multiprocessing_is_available: + return 'processes' + return 'threads' def baseline_path(self): """Return the absolute path to the directory to store new baselines @@ -323,10 +326,17 @@ class Port(object): def expected_checksum(self, test): """Returns the checksum of the image we expect the test to produce, or None if it is a text-only test.""" - path = self.expected_filename(test, '.checksum') - if not self.path_exists(path): - return None - return self._filesystem.read_binary_file(path) + png_path = self.expected_filename(test, '.png') + checksum_path = self._filesystem.splitext(png_path)[0] + '.checksum' + + if self.path_exists(checksum_path): + return self._filesystem.read_binary_file(checksum_path) + + if self.path_exists(png_path): + with self._filesystem.open_binary_file_for_reading(png_path) as filehandle: + return read_checksum_from_png.read_checksum(filehandle) + + return None def expected_image(self, test): """Returns the image we expect the test to produce.""" @@ -347,6 +357,14 @@ class Port(object): text = self._filesystem.read_binary_file(path) return text.replace("\r\n", "\n") + def reftest_expected_filename(self, filename): + """Return the filename of reference we expect the test matches.""" + return self.expected_filename(filename, '.html') + + def reftest_expected_mismatch_filename(self, filename): + """Return the filename of reference we don't expect the test matches.""" + return self.expected_filename(filename, '-mismatch.html') + def filename_to_uri(self, filename): """Convert a test file (which is an absolute path) to a URI.""" LAYOUTTEST_HTTP_DIR = "http/tests/" @@ -498,6 +516,9 @@ class Port(object): def script_path(self, script_name): return self._config.script_path(script_name) + def script_shell_command(self, script_name): + return self._config.script_shell_command(script_name) + def path_to_test_expectations_file(self): """Update the test expectations to the passed-in string. diff --git a/Tools/Scripts/webkitpy/layout_tests/port/chromium_linux.py b/Tools/Scripts/webkitpy/layout_tests/port/chromium_linux.py index a8e1bb2..2cd2435 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/chromium_linux.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/chromium_linux.py @@ -114,11 +114,6 @@ class ChromiumLinuxPort(chromium.ChromiumPort): 'LinuxBuildInstructions') return result - def default_worker_model(self): - if self._multiprocessing_is_available: - return 'processes' - return 'old-threads' - def test_platform_name(self): # We use 'linux' instead of 'chromium-linux' in test_expectations.txt. return 'linux' diff --git a/Tools/Scripts/webkitpy/layout_tests/port/chromium_mac.py b/Tools/Scripts/webkitpy/layout_tests/port/chromium_mac.py index 78a6682..141b587 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/chromium_mac.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/chromium_mac.py @@ -46,11 +46,25 @@ class ChromiumMacPort(chromium.ChromiumPort): SUPPORTED_OS_VERSIONS = ('leopard', 'snowleopard') FALLBACK_PATHS = { - 'leopard': ['chromium-mac-leopard', 'chromium-mac-snowleopard', 'chromium-mac', 'chromium', - 'mac-leopard', 'mac-snowleopard', 'mac'], - 'snowleopard': ['chromium-mac-snowleopard', 'chromium-mac', 'chromium', - 'mac-snowleopard', 'mac'], - '': ['chromium-mac', 'chromium', 'mac'], + 'leopard': [ + 'chromium-mac-leopard', + 'chromium-mac', + 'chromium', + 'mac-leopard', + 'mac-snowleopard', + 'mac', + ], + 'snowleopard': [ + 'chromium-mac', + 'chromium', + 'mac-snowleopard', + 'mac', + ], + '': [ + 'chromium-mac', + 'chromium', + 'mac', + ], } def __init__(self, port_name=None, os_version_string=None, rebaselining=False, **kwargs): @@ -100,22 +114,12 @@ class ChromiumMacPort(chromium.ChromiumPort): return result def default_child_processes(self): - if self.get_option('worker_model') == 'old-threads': - # FIXME: we need to run single-threaded for now. See - # https://bugs.webkit.org/show_bug.cgi?id=38553. Unfortunately this - # routine is called right before the logger is configured, so if we - # try to _log.warning(), it gets thrown away. - import sys - sys.stderr.write("Defaulting to one child - see https://bugs.webkit.org/show_bug.cgi?id=38553\n") + if not self._multiprocessing_is_available: + # Running multiple threads in Mac Python is unstable (See + # https://bugs.webkit.org/show_bug.cgi?id=38553 for more info). return 1 - return chromium.ChromiumPort.default_child_processes(self) - def default_worker_model(self): - if self._multiprocessing_is_available: - return 'processes' - return 'old-threads' - def driver_name(self): return "DumpRenderTree" diff --git a/Tools/Scripts/webkitpy/layout_tests/port/chromium_win.py b/Tools/Scripts/webkitpy/layout_tests/port/chromium_win.py index e7c6e49..d0908df 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/chromium_win.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/chromium_win.py @@ -129,6 +129,10 @@ class ChromiumWinPort(chromium.ChromiumPort): 'build-instructions-windows') return result + def default_worker_model(self): + # FIXME: should use base class method instead. See bug 55163. + return 'old-threads' + def relative_test_filename(self, filename): path = filename[len(self.layout_tests_dir()) + 1:] return path.replace('\\', '/') diff --git a/Tools/Scripts/webkitpy/layout_tests/port/dryrun.py b/Tools/Scripts/webkitpy/layout_tests/port/dryrun.py index 6b3bd51..20aa776 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/dryrun.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/dryrun.py @@ -118,12 +118,26 @@ class DryrunDriver(base.Driver): def run_test(self, driver_input): start_time = time.time() - text_output = self._port.expected_text(driver_input.filename) - - if driver_input.image_hash is not None: + fs = self._port._filesystem + if fs.exists(self._port.reftest_expected_filename(driver_input.filename)) or \ + fs.exists(self._port.reftest_expected_mismatch_filename(driver_input.filename)): + text_output = 'test-text' + image = 'test-image' + hash = 'test-checksum' + elif driver_input.filename.endswith('-expected.html'): + text_output = 'test-text' + image = 'test-image' + hash = 'test-checksum' + elif driver_input.filename.endswith('-expected-mismatch.html'): + text_output = 'test-text-mismatch' + image = 'test-image-mismatch' + hash = 'test-checksum-mismatch' + elif driver_input.image_hash is not None: + text_output = self._port.expected_text(driver_input.filename) image = self._port.expected_image(driver_input.filename) hash = self._port.expected_checksum(driver_input.filename) else: + text_output = self._port.expected_text(driver_input.filename) image = None hash = None return base.DriverOutput(text_output, image, hash, False, diff --git a/Tools/Scripts/webkitpy/layout_tests/port/http_server.py b/Tools/Scripts/webkitpy/layout_tests/port/http_server.py index 752b099..1753aee 100755 --- a/Tools/Scripts/webkitpy/layout_tests/port/http_server.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/http_server.py @@ -55,7 +55,7 @@ class HttpdNotStarted(Exception): class Lighttpd(http_server_base.HttpServerBase): def __init__(self, port_obj, output_dir, background=False, port=None, - root=None, run_background=None): + root=None, run_background=None, layout_tests_dir=None): """Args: output_dir: the absolute path to the layout test result directory """ @@ -66,16 +66,21 @@ class Lighttpd(http_server_base.HttpServerBase): self._port = port self._root = root self._run_background = run_background + self._layout_tests_dir = layout_tests_dir + if self._port: self._port = int(self._port) + if not self._layout_tests_dir: + self._layout_tests_dir = self._port_obj.layout_tests_dir() + try: self._webkit_tests = os.path.join( - self._port_obj.layout_tests_dir(), 'http', 'tests') + self._layout_tests_dir, 'http', 'tests') self._js_test_resource = os.path.join( - self._port_obj.layout_tests_dir(), 'fast', 'js', 'resources') + self._layout_tests_dir, 'fast', 'js', 'resources') self._media_resource = os.path.join( - self._port_obj.layout_tests_dir(), 'media') + self._layout_tests_dir, 'media') except: self._webkit_tests = None diff --git a/Tools/Scripts/webkitpy/layout_tests/port/mac.py b/Tools/Scripts/webkitpy/layout_tests/port/mac.py index 0168ec7..4315543 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/mac.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/mac.py @@ -88,14 +88,14 @@ class MacPort(WebKitPort): # four threads in parallel. # See https://bugs.webkit.org/show_bug.cgi?id=36622 child_processes = WebKitPort.default_child_processes(self) - if self.get_option('worker_model') == 'old-threads' and child_processes > 4: + if not self._multiprocessing_is_available and child_processes > 4: return 4 return child_processes def default_worker_model(self): if self._multiprocessing_is_available: return 'processes' - return 'old-threads' + return 'threads' def baseline_search_path(self): return map(self._webkit_baseline_path, self.FALLBACK_PATHS[self._version]) diff --git a/Tools/Scripts/webkitpy/layout_tests/port/mock_drt_unittest.py b/Tools/Scripts/webkitpy/layout_tests/port/mock_drt_unittest.py index 71de14b..b6f6e8a 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/mock_drt_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/mock_drt_unittest.py @@ -207,6 +207,9 @@ class MockDRTTest(unittest.TestCase): def test_textonly(self): self.assertTest('passes/image.html', False) + def test_checksum_in_png(self): + self.assertTest('passes/checksum_in_image.html', True) + class MockChromiumDRTTest(MockDRTTest): def extra_args(self, pixel_tests): diff --git a/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py b/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py index d37fdc0..649e33c 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py @@ -73,7 +73,7 @@ class PortTestCase(unittest.TestCase): if multiprocessing: self.assertEqual(port.default_worker_model(), 'processes') else: - self.assertEqual(port.default_worker_model(), 'old-threads') + self.assertEqual(port.default_worker_model(), 'threads') def test_driver_cmd_line(self): port = self.make_port() diff --git a/Tools/Scripts/webkitpy/layout_tests/port/test.py b/Tools/Scripts/webkitpy/layout_tests/port/test.py index d323ed5..392818d 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/test.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/test.py @@ -50,6 +50,7 @@ class TestInstance: self.keyboard = False self.error = '' self.timeout = False + self.is_reftest = False # The values of each field are treated as raw byte strings. They # will be converted to unicode strings where appropriate using @@ -78,6 +79,13 @@ class TestList: test.__dict__[key] = value self.tests[name] = test + def add_reftest(self, name, reference_name, same_image): + self.add(name, actual_checksum='xxx', actual_image='XXX', is_reftest=True) + if same_image: + self.add(reference_name, actual_checksum='xxx', actual_image='XXX', is_reftest=True) + else: + self.add(reference_name, actual_checksum='yyy', actual_image='YYY', is_reftest=True) + def keys(self): return self.tests.keys() @@ -104,7 +112,9 @@ def unit_test_list(): actual_checksum='image_checksum_fail-checksum', actual_image='image_checksum_fail-png') tests.add('failures/expected/keyboard.html', keyboard=True) - tests.add('failures/expected/missing_check.html', expected_checksum=None) + tests.add('failures/expected/missing_check.html', + expected_checksum=None, + expected_image=None) tests.add('failures/expected/missing_image.html', expected_image=None) tests.add('failures/expected/missing_text.html', expected_text=None) tests.add('failures/expected/newlines_leading.html', @@ -120,15 +130,29 @@ def unit_test_list(): actual_checksum='text-image-checksum_fail-checksum') tests.add('failures/unexpected/timeout.html', timeout=True) tests.add('http/tests/passes/text.html') + tests.add('http/tests/passes/image.html') tests.add('http/tests/ssl/text.html') tests.add('passes/error.html', error='stuff going to stderr') tests.add('passes/image.html') tests.add('passes/platform_image.html') + tests.add('passes/checksum_in_image.html', + expected_checksum=None, + expected_image='tEXtchecksum\x00checksum_in_image-checksum') # Text output files contain "\r\n" on Windows. This may be # helpfully filtered to "\r\r\n" by our Python/Cygwin tooling. tests.add('passes/text.html', expected_text='\nfoo\n\n', actual_text='\nfoo\r\n\r\r\n') + + # For reftests. + tests.add_reftest('passes/reftest.html', 'passes/reftest-expected.html', same_image=True) + tests.add_reftest('passes/mismatch.html', 'passes/mismatch-expected-mismatch.html', same_image=False) + tests.add_reftest('failures/expected/reftest.html', 'failures/expected/reftest-expected.html', same_image=False) + tests.add_reftest('failures/expected/mismatch.html', 'failures/expected/mismatch-expected-mismatch.html', same_image=True) + tests.add_reftest('failures/unexpected/reftest.html', 'failures/unexpected/reftest-expected.html', same_image=False) + tests.add_reftest('failures/unexpected/mismatch.html', 'failures/unexpected/mismatch-expected-mismatch.html', same_image=True) + # FIXME: Add a reftest which crashes. + tests.add('websocket/tests/passes/text.html') return tests @@ -158,6 +182,8 @@ def unit_test_filesystem(files=None): # Add each test and the expected output, if any. for test in test_list.tests.values(): add_file(files, test, '.html', '') + if test.is_reftest: + continue add_file(files, test, '-expected.txt', test.expected_text) add_file(files, test, '-expected.checksum', test.expected_checksum) add_file(files, test, '-expected.png', test.expected_image) @@ -169,12 +195,14 @@ WONTFIX : failures/expected/crash.html = CRASH // This one actually passes because the checksums will match. WONTFIX : failures/expected/image.html = PASS WONTFIX : failures/expected/image_checksum.html = IMAGE +WONTFIX : failures/expected/mismatch.html = IMAGE WONTFIX : failures/expected/missing_check.html = MISSING PASS WONTFIX : failures/expected/missing_image.html = MISSING PASS WONTFIX : failures/expected/missing_text.html = MISSING PASS WONTFIX : failures/expected/newlines_leading.html = TEXT WONTFIX : failures/expected/newlines_trailing.html = TEXT WONTFIX : failures/expected/newlines_with_excess_CR.html = TEXT +WONTFIX : failures/expected/reftest.html = IMAGE WONTFIX : failures/expected/text.html = TEXT WONTFIX : failures/expected/timeout.html = TIMEOUT WONTFIX SKIP : failures/expected/hang.html = TIMEOUT @@ -222,6 +250,12 @@ class TestPort(base.Port): def baseline_search_path(self): return [self.baseline_path()] + def default_child_processes(self): + return 1 + + def default_worker_model(self): + return 'inline' + def check_build(self, needs_http): return True diff --git a/Tools/Scripts/webkitpy/layout_tests/port/test_files.py b/Tools/Scripts/webkitpy/layout_tests/port/test_files.py index 534462a..fbbbea5 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/test_files.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/test_files.py @@ -105,11 +105,10 @@ def _has_supported_extension(filesystem, filename): return extension in _supported_file_extensions -def _is_reference_html_file(filename): +def is_reference_html_file(filename): """Return true if the filename points to a reference HTML file.""" if (filename.endswith('-expected.html') or filename.endswith('-expected-mismatch.html')): - _log.warn("Reftests are not supported - ignoring %s" % filename) return True return False @@ -117,4 +116,4 @@ def _is_reference_html_file(filename): def _is_test_file(filesystem, dirname, filename): """Return true if the filename points to a test file.""" return (_has_supported_extension(filesystem, filename) and - not _is_reference_html_file(filename)) + not is_reference_html_file(filename)) diff --git a/Tools/Scripts/webkitpy/layout_tests/read_checksum_from_png.py b/Tools/Scripts/webkitpy/layout_tests/read_checksum_from_png.py new file mode 100644 index 0000000..70a0502 --- /dev/null +++ b/Tools/Scripts/webkitpy/layout_tests/read_checksum_from_png.py @@ -0,0 +1,40 @@ +#!/usr/bin/env python +# Copyright (c) 2011 Google Inc. All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + + +def read_checksum(filehandle): + # We expect the comment to be at the beginning of the file. + data = filehandle.read(2048) + comment_key = 'tEXtchecksum\x00' + comment_pos = data.find(comment_key) + if comment_pos == -1: + return + + checksum_pos = comment_pos + len(comment_key) + return data[checksum_pos:checksum_pos + 32] diff --git a/Tools/Scripts/webkitpy/layout_tests/read_checksum_from_png_unittest.py b/Tools/Scripts/webkitpy/layout_tests/read_checksum_from_png_unittest.py new file mode 100644 index 0000000..7375741 --- /dev/null +++ b/Tools/Scripts/webkitpy/layout_tests/read_checksum_from_png_unittest.py @@ -0,0 +1,44 @@ +#!/usr/bin/env python +# Copyright (C) 2011 Google Inc. All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY +# EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED +# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY +# DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES +# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF +# THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +import StringIO +import unittest +from webkitpy.layout_tests import read_checksum_from_png + + +class ReadChecksumFromPngTest(unittest.TestCase): + def test_read_checksum(self): + # Test a file with the comment. + filehandle = StringIO.StringIO('''\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x03 \x00\x00\x02X\x08\x02\x00\x00\x00\x15\x14\x15'\x00\x00\x00)tEXtchecksum\x003c4134fe2739880353f91c5b84cadbaaC\xb8?\xec\x00\x00\x16\xfeIDATx\x9c\xed\xdd[\x8cU\xe5\xc1\xff\xf15T\x18\x0ea,)\xa6\x80XZ<\x10\n\xd6H\xc4V\x88}\xb5\xa9\xd6r\xd5\x0bki0\xa6\xb5ih\xd2\xde\x98PHz\xd1\x02=\\q#\x01\x8b\xa5rJ\x8b\x88i\xacM\xc5h\x8cbMk(\x1ez@!\x0c\xd5\xd2\xc2\xb44\x1c\x848\x1dF(\xeb\x7f\xb1\xff\xd9\xef~g\xd6\xde3\xe0o\x10\xec\xe7sa6{\xd6z\xd6\xb3\xd7\xf3\xa8_7\xdbM[Y\x96\x05\x00\x009\xc3\xde\xeb\t\x00\x00\xbc\xdf\x08,\x00\x800\x81\x05\x00\x10&\xb0\x00\x00\xc2\x04\x16\x00@\x98\xc0\x02\x00\x08\x13X\x00\x00a\x02\x0b\x00 L x01\x00\x84\t,\x00\x800\x81\x05\x00\x10\xd64\xb0\xda\x9a\xdb\xb6m\xdb\xb4i\xd3\xfa\x9fr\xf3\xcd7\x0f\xe5T\x07\xe5\xd4\xa9''') + checksum = read_checksum_from_png.read_checksum(filehandle) + self.assertEquals('3c4134fe2739880353f91c5b84cadbaa', checksum) + + # Test a file without the comment. + filehandle = StringIO.StringIO('''\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x03 \x00\x00\x02X\x08\x02\x00\x00\x00\x15\x14\x15'\x00\x00\x16\xfeIDATx\x9c\xed\xdd[\x8cU\xe5\xc1\xff\xf15T\x18\x0ea,)\xa6\x80XZ<\x10\n\xd6H\xc4V\x88}\xb5\xa9\xd6r\xd5\x0bki0\xa6\xb5ih\xd2\xde\x98PHz\xd1\x02=\\q#\x01\x8b\xa5rJ\x8b\x88i\xacM\xc5h\x8cbMk(\x1ez@!\x0c\xd5\xd2\xc2\xb44\x1c\x848\x1dF(\xeb\x7f\xb1\xff\xd9\xef~g\xd6\xde3\xe0o\x10\xec\xe7sa6{\xd6z\xd6\xb3\xd7\xf3\xa8_7\xdbM[Y\x96\x05\x00\x009\xc3\xde\xeb\t\x00\x00\xbc\xdf\x08,\x00\x800\x81\x05\x00\x10&\xb0\x00\x00\xc2\x04\x16\x00@\x98\xc0\x02\x00\x08\x13X\x00\x00a\x02\x0b\x00 L x01\x00\x84\t,\x00\x800\x81\x05\x00\x10\xd64\xb0\xda\x9a\xdb\xb6m\xdb\xb4i\xd3\xfa\x9fr\xf3\xcd7\x0f\xe5T\x07\xe5\xd4\xa9S\x8b\x17/\x1e?~\xfc\xf8\xf1\xe3\xef\xbf\xff\xfe\xf7z:M5\xbb\x87\x17\xcbUZ\x8f|V\xd7\xbd\x10\xb6\xcd{b\x88\xf6j\xb3\x9b?\x14\x9b\xa1>\xe6\xf9\xd9\xcf\x00\x17\x93''') + checksum = read_checksum_from_png.read_checksum(filehandle) + self.assertEquals(None, checksum) + + +if __name__ == '__main__': + unittest.main() diff --git a/Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py b/Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py index 24b8c97..9f1d347 100644 --- a/Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py +++ b/Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py @@ -59,8 +59,8 @@ from webkitpy.layout_tests.layout_package import test_expectations _log = logging.getLogger(__name__) -BASELINE_SUFFIXES = ['.txt', '.png', '.checksum'] -REBASELINE_PLATFORM_ORDER = ['mac', 'win', 'win-xp', 'win-vista', 'linux'] +BASELINE_SUFFIXES = ('.txt', '.png', '.checksum') +REBASELINE_PLATFORM_ORDER = ('mac', 'win', 'win-xp', 'win-vista', 'linux') ARCHIVE_DIR_NAME_DICT = {'win': 'Webkit_Win__deps_', 'win-vista': 'webkit-dbg-vista', 'win-xp': 'Webkit_Win__deps_', @@ -171,7 +171,7 @@ class Rebaseliner(object): self._rebaseline_port = port.get( self._target_port.test_platform_name_to_name(platform), options, filesystem=self._filesystem, rebaselining=True) - self._rebaselining_tests = [] + self._rebaselining_tests = set() self._rebaselined_tests = [] # Create tests and expectations helper which is used to: @@ -179,12 +179,8 @@ class Rebaseliner(object): # -. update the tests in test_expectations file after rebaseline # is done. expectations_str = self._rebaseline_port.test_expectations() - self._test_expectations = \ - test_expectations.TestExpectations(self._rebaseline_port, - None, - expectations_str, - self._rebaseline_port.test_configuration(), - False) + self._test_expectations = test_expectations.TestExpectations( + self._rebaseline_port, None, expectations_str, self._rebaseline_port.test_configuration(), False) self._url_fetcher = url_fetcher self._zip_factory = zip_factory self._scm = scm @@ -194,6 +190,8 @@ class Rebaseliner(object): log_dashed_string('Compiling rebaselining tests', self._platform) if not self._compile_rebaselining_tests(): + return False + if not self.get_rebaselining_tests(): return True log_dashed_string('Downloading archive', self._platform) @@ -203,30 +201,24 @@ class Rebaseliner(object): _log.error('No archive found.') return False - log_dashed_string('Extracting and adding new baselines', - self._platform) + log_dashed_string('Extracting and adding new baselines', self._platform) if not self._extract_and_add_new_baselines(archive_file): archive_file.close() return False archive_file.close() - log_dashed_string('Updating rebaselined tests in file', - self._platform) + log_dashed_string('Updating rebaselined tests in file', self._platform) self._update_rebaselined_tests_in_file(backup) _log.info('') if len(self._rebaselining_tests) != len(self._rebaselined_tests): - _log.warning('NOT ALL TESTS THAT NEED REBASELINING HAVE BEEN ' - 'REBASELINED.') - _log.warning(' Total tests needing rebaselining: %d', - len(self._rebaselining_tests)) - _log.warning(' Total tests rebaselined: %d', - len(self._rebaselined_tests)) + _log.warning('NOT ALL TESTS THAT NEED REBASELINING HAVE BEEN REBASELINED.') + _log.warning(' Total tests needing rebaselining: %d', len(self._rebaselining_tests)) + _log.warning(' Total tests rebaselined: %d', len(self._rebaselined_tests)) return False - _log.warning('All tests needing rebaselining were successfully ' - 'rebaselined.') + _log.warning('All tests needing rebaselining were successfully rebaselined.') return True @@ -237,26 +229,33 @@ class Rebaseliner(object): """Compile list of tests that need rebaselining for the platform. Returns: - List of tests that need rebaselining or - None if there is no such test. + False if reftests are wrongly marked as 'needs rebaselining' or True """ self._rebaselining_tests = \ self._test_expectations.get_rebaselining_failures() if not self._rebaselining_tests: _log.warn('No tests found that need rebaselining.') - return None + return True + + fs = self._target_port._filesystem + for test in self._rebaselining_tests: + test_abspath = self._target_port.abspath_for_test(test) + if (fs.exists(self._target_port.reftest_expected_filename(test_abspath)) or + fs.exists(self._target_port.reftest_expected_mismatch_filename(test_abspath))): + _log.error('%s seems to be a reftest. We can not rebase for reftests.', test) + self._rebaselining_tests = set() + return False - _log.info('Total number of tests needing rebaselining ' - 'for "%s": "%d"', self._platform, - len(self._rebaselining_tests)) + _log.info('Total number of tests needing rebaselining for "%s": "%d"', + self._platform, len(self._rebaselining_tests)) test_no = 1 for test in self._rebaselining_tests: _log.info(' %d: %s', test_no, test) test_no += 1 - return self._rebaselining_tests + return True def _get_latest_revision(self, url): """Get the latest layout test revision number from buildbot. @@ -324,8 +323,7 @@ class Rebaseliner(object): latest_revision = self._get_latest_revision(url_base) if latest_revision is None or latest_revision <= 0: return None - archive_url = ('%s%s/layout-test-results.zip' % (url_base, - latest_revision)) + archive_url = '%s%s/layout-test-results.zip' % (url_base, latest_revision) _log.info('Archive url: "%s"', archive_url) return archive_url @@ -336,7 +334,7 @@ class Rebaseliner(object): return None archive_file = zipfileset.ZipFileSet(url, filesystem=self._filesystem, - zip_factory=self._zip_factory) + zip_factory=self._zip_factory) _log.info('Archive downloaded') return archive_file @@ -344,92 +342,83 @@ class Rebaseliner(object): """Extract new baselines from the zip file and add them to SVN repository. Returns: - List of tests that have been rebaselined or - None on failure. - """ - + List of tests that have been rebaselined or None on failure.""" zip_namelist = zip_file.namelist() _log.debug('zip file namelist:') for name in zip_namelist: _log.debug(' ' + name) - platform = self._rebaseline_port.test_platform_name_to_name( - self._platform) + platform = self._rebaseline_port.test_platform_name_to_name(self._platform) _log.debug('Platform dir: "%s"', platform) - test_no = 1 self._rebaselined_tests = [] - for test in self._rebaselining_tests: - _log.info('Test %d: %s', test_no, test) - - found = False - scm_error = False - test_basename = self._filesystem.splitext(test)[0] - for suffix in BASELINE_SUFFIXES: - archive_test_name = ('layout-test-results/%s-actual%s' % - (test_basename, suffix)) - _log.debug(' Archive test file name: "%s"', - archive_test_name) - if not archive_test_name in zip_namelist: - _log.info(' %s file not in archive.', suffix) - continue - - found = True - _log.info(' %s file found in archive.', suffix) - - # Extract new baseline from archive and save it to a temp file. - data = zip_file.read(archive_test_name) - tempfile, temp_name = self._filesystem.open_binary_tempfile(suffix) - tempfile.write(data) - tempfile.close() - - expected_filename = '%s-expected%s' % (test_basename, suffix) - expected_fullpath = self._filesystem.join( - self._rebaseline_port.baseline_path(), expected_filename) - expected_fullpath = self._filesystem.normpath(expected_fullpath) - _log.debug(' Expected file full path: "%s"', - expected_fullpath) - - # TODO(victorw): for now, the rebaselining tool checks whether - # or not THIS baseline is duplicate and should be skipped. - # We could improve the tool to check all baselines in upper - # and lower - # levels and remove all duplicated baselines. - if self._is_dup_baseline(temp_name, - expected_fullpath, - test, - suffix, - self._platform): - self._filesystem.remove(temp_name) - self._delete_baseline(expected_fullpath) - continue - - self._filesystem.maybe_make_directory(self._filesystem.dirname(expected_fullpath)) - - self._filesystem.move(temp_name, expected_fullpath) - - if 0 != self._scm.add(expected_fullpath, return_exit_code=True): - # FIXME: print detailed diagnose messages - scm_error = True - elif suffix != '.checksum': - self._create_html_baseline_files(expected_fullpath) - - if not found: - _log.warn(' No new baselines found in archive.') - else: - if scm_error: - _log.warn(' Failed to add baselines to your repository.') - else: - _log.info(' Rebaseline succeeded.') - self._rebaselined_tests.append(test) - - test_no += 1 + for test_no, test in enumerate(self._rebaselining_tests): + _log.info('Test %d: %s', test_no + 1, test) + self._extract_and_add_new_baseline(test, zip_file) zip_file.close() return self._rebaselined_tests + def _extract_and_add_new_baseline(self, test, zip_file): + found = False + scm_error = False + test_basename = self._filesystem.splitext(test)[0] + for suffix in BASELINE_SUFFIXES: + archive_test_name = 'layout-test-results/%s-actual%s' % (test_basename, suffix) + _log.debug(' Archive test file name: "%s"', archive_test_name) + if not archive_test_name in zip_file.namelist(): + _log.info(' %s file not in archive.', suffix) + continue + + found = True + _log.info(' %s file found in archive.', suffix) + + temp_name = self._extract_from_zip_to_tempfile(zip_file, archive_test_name) + + expected_filename = '%s-expected%s' % (test_basename, suffix) + expected_fullpath = self._filesystem.join( + self._rebaseline_port.baseline_path(), expected_filename) + expected_fullpath = self._filesystem.normpath(expected_fullpath) + _log.debug(' Expected file full path: "%s"', expected_fullpath) + + # TODO(victorw): for now, the rebaselining tool checks whether + # or not THIS baseline is duplicate and should be skipped. + # We could improve the tool to check all baselines in upper + # and lower levels and remove all duplicated baselines. + if self._is_dup_baseline(temp_name, expected_fullpath, test, suffix, self._platform): + self._filesystem.remove(temp_name) + self._delete_baseline(expected_fullpath) + continue + + self._filesystem.maybe_make_directory(self._filesystem.dirname(expected_fullpath)) + self._filesystem.move(temp_name, expected_fullpath) + + if self._scm.add(expected_fullpath, return_exit_code=True): + # FIXME: print detailed diagnose messages + scm_error = True + elif suffix != '.checksum': + self._create_html_baseline_files(expected_fullpath) + + if not found: + _log.warn(' No new baselines found in archive.') + elif scm_error: + _log.warn(' Failed to add baselines to your repository.') + else: + _log.info(' Rebaseline succeeded.') + self._rebaselined_tests.append(test) + + def _extract_from_zip_to_tempfile(self, zip_file, filename): + """Extracts |filename| from |zip_file|, a ZipFileSet. Returns the full + path name to the extracted file.""" + data = zip_file.read(filename) + suffix = self._filesystem.splitext(filename)[1] + tempfile, temp_name = self._filesystem.open_binary_tempfile(suffix) + tempfile.write(data) + tempfile.close() + return temp_name + def _is_dup_baseline(self, new_baseline, baseline_path, test, suffix, platform): """Check whether a baseline is duplicate and can fallback to same @@ -448,25 +437,26 @@ class Rebaseliner(object): True if the baseline is unnecessary. False otherwise. """ - test_filepath = self._filesystem.join(self._target_port.layout_tests_dir(), - test) + test_filepath = self._filesystem.join(self._target_port.layout_tests_dir(), test) all_baselines = self._rebaseline_port.expected_baselines( test_filepath, suffix, True) - for (fallback_dir, fallback_file) in all_baselines: - if fallback_dir and fallback_file: - fallback_fullpath = self._filesystem.normpath( - self._filesystem.join(fallback_dir, fallback_file)) - if fallback_fullpath.lower() != baseline_path.lower(): - new_output = self._filesystem.read_binary_file(new_baseline) - fallback_output = self._filesystem.read_binary_file(fallback_fullpath) - is_image = baseline_path.lower().endswith('.png') - if not self._diff_baselines(new_output, fallback_output, - is_image): - _log.info(' Found same baseline at %s', - fallback_fullpath) - return True - else: - return False + + for fallback_dir, fallback_file in all_baselines: + if not fallback_dir or not fallback_file: + continue + + fallback_fullpath = self._filesystem.normpath( + self._filesystem.join(fallback_dir, fallback_file)) + if fallback_fullpath.lower() == baseline_path.lower(): + continue + + new_output = self._filesystem.read_binary_file(new_baseline) + fallback_output = self._filesystem.read_binary_file(fallback_fullpath) + is_image = baseline_path.lower().endswith('.png') + if not self._diff_baselines(new_output, fallback_output, is_image): + _log.info(' Found same baseline at %s', fallback_fullpath) + return True + return False return False @@ -483,8 +473,8 @@ class Rebaseliner(object): if is_image: return self._port.diff_image(output1, output2, None) - else: - return self._port.compare_text(output1, output2) + + return self._port.compare_text(output1, output2) def _delete_baseline(self, filename): """Remove the file from repository and delete it from disk. @@ -508,14 +498,12 @@ class Rebaseliner(object): """ if self._rebaselined_tests: - new_expectations = ( - self._test_expectations.remove_platform_from_expectations( - self._rebaselined_tests, self._platform)) + new_expectations = self._test_expectations.remove_platform_from_expectations( + self._rebaselined_tests, self._platform) path = self._target_port.path_to_test_expectations_file() if backup: - date_suffix = time.strftime('%Y%m%d%H%M%S', - time.localtime(time.time())) - backup_file = ('%s.orig.%s' % (path, date_suffix)) + date_suffix = time.strftime('%Y%m%d%H%M%S', time.localtime(time.time())) + backup_file = '%s.orig.%s' % (path, date_suffix) if self._filesystem.exists(backup_file): self._filesystem.remove(backup_file) _log.info('Saving original file to "%s"', backup_file) @@ -541,8 +529,7 @@ class Rebaseliner(object): # Copy the new baseline to html directory for result comparison. baseline_filename = self._filesystem.basename(baseline_fullpath) new_file = get_result_file_fullpath(self._filesystem, self._options.html_directory, - baseline_filename, self._platform, - 'new') + baseline_filename, self._platform, 'new') self._filesystem.copyfile(baseline_fullpath, new_file) _log.info(' Html: copied new baseline file from "%s" to "%s".', baseline_fullpath, new_file) @@ -554,19 +541,16 @@ class Rebaseliner(object): _log.info(e) output = "" - if (not output) or (output.upper().rstrip().endswith( - 'NO SUCH FILE OR DIRECTORY')): + if (not output) or (output.upper().rstrip().endswith('NO SUCH FILE OR DIRECTORY')): _log.info(' No base file: "%s"', baseline_fullpath) return base_file = get_result_file_fullpath(self._filesystem, self._options.html_directory, - baseline_filename, self._platform, - 'old') + baseline_filename, self._platform, 'old') if base_file.upper().endswith('.PNG'): self._filesystem.write_binary_file(base_file, output) else: self._filesystem.write_text_file(base_file, output) - _log.info(' Html: created old baseline file: "%s".', - base_file) + _log.info(' Html: created old baseline file: "%s".', base_file) # Get the diff between old and new baselines and save to the html dir. if baseline_filename.upper().endswith('.TXT'): @@ -576,8 +560,7 @@ class Rebaseliner(object): self._options.html_directory, baseline_filename, self._platform, 'diff') self._filesystem.write_text_file(diff_file, output) - _log.info(' Html: created baseline diff file: "%s".', - diff_file) + _log.info(' Html: created baseline diff file: "%s".', diff_file) class HtmlGenerator(object): @@ -663,8 +646,7 @@ class HtmlGenerator(object): _log.debug(html) self._filesystem.write_text_file(self._html_file, html) - _log.info('Baseline comparison html generated at "%s"', - self._html_file) + _log.info('Baseline comparison html generated at "%s"', self._html_file) def show_html(self): """Launch the rebaselining html in brwoser.""" @@ -716,8 +698,7 @@ class HtmlGenerator(object): 'name': baseline_filename} diff_file = get_result_file_fullpath(self._filesystem, self._html_directory, - baseline_filename, platform, - 'diff') + baseline_filename, platform, 'diff') _log.info(' Baseline diff file: "%s"', diff_file) if self._filesystem.exists(diff_file): links += html_td_link % {'uri': self.abspath_to_uri(diff_file), @@ -747,11 +728,9 @@ class HtmlGenerator(object): _log.info(' Checking %s files', suffix) for platform in self._platforms: - links = self._generate_baseline_links(test_basename, suffix, - platform) + links = self._generate_baseline_links(test_basename, suffix, platform) if links: - row = self.HTML_TD_NOLINK % self._get_baseline_result_type( - suffix) + row = self.HTML_TD_NOLINK % self._get_baseline_result_type(suffix) row += self.HTML_TD_NOLINK % platform row += links _log.debug(' html row: %s', row) diff --git a/Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py b/Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py index c50e1c4..7179bb7 100644 --- a/Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py @@ -174,6 +174,20 @@ class TestRebaseliner(unittest.TestCase): rebaseliner.run(False) self.assertEqual(len(filesystem.written_files), 1) + def test_rebaselining_tests(self): + rebaseliner, filesystem = self.make_rebaseliner( + "BUGX REBASELINE MAC : failures/expected/image.html = IMAGE") + compile_success = rebaseliner._compile_rebaselining_tests() + self.assertTrue(compile_success) + self.assertEqual(set(['failures/expected/image.html']), rebaseliner.get_rebaselining_tests()) + + def test_rebaselining_tests_should_ignore_reftests(self): + rebaseliner, filesystem = self.make_rebaseliner( + "BUGX REBASELINE : failures/expected/reftest.html = IMAGE") + compile_success = rebaseliner._compile_rebaselining_tests() + self.assertFalse(compile_success) + self.assertFalse(rebaseliner.get_rebaselining_tests()) + def test_one_platform(self): rebaseliner, filesystem = self.make_rebaseliner( "BUGX REBASELINE MAC : failures/expected/image.html = IMAGE") diff --git a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py index 3fe7b14..7076ef2 100644 --- a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py @@ -55,6 +55,7 @@ from webkitpy.layout_tests import port from webkitpy.layout_tests import run_webkit_tests from webkitpy.layout_tests.layout_package import dump_render_tree_thread from webkitpy.layout_tests.port.test import TestPort, TestDriver +from webkitpy.layout_tests.port.test_files import is_reference_html_file from webkitpy.python24.versioning import compare_version from webkitpy.test.skip import skip_if @@ -72,8 +73,8 @@ def parse_args(extra_args=None, record_results=False, tests_included=False, args.extend(['--platform', 'test']) if not record_results: args.append('--no-record-results') - if not '--child-processes' in extra_args: - args.extend(['--worker-model', 'old-inline']) + if not '--child-processes' in extra_args and not '--worker-model' in extra_args: + args.extend(['--worker-model', 'inline']) args.extend(extra_args) if not tests_included: # We use the glob to test that globbing works. @@ -124,7 +125,8 @@ def run_and_capture(port_obj, options, parsed_args): return (res, buildbot_output, regular_output) -def get_tests_run(extra_args=None, tests_included=False, flatten_batches=False, filesystem=None): +def get_tests_run(extra_args=None, tests_included=False, flatten_batches=False, + filesystem=None, include_reference_html=False): extra_args = extra_args or [] if not tests_included: # Not including http tests since they get run out of order (that @@ -136,6 +138,7 @@ def get_tests_run(extra_args=None, tests_included=False, flatten_batches=False, test_batches = [] + class RecordingTestDriver(TestDriver): def __init__(self, port, worker_number): TestDriver.__init__(self, port, worker_number) @@ -153,7 +156,11 @@ def get_tests_run(extra_args=None, tests_included=False, flatten_batches=False, self._current_test_batch = [] test_batches.append(self._current_test_batch) test_name = self._port.relative_test_filename(test_input.filename) - self._current_test_batch.append(test_name) + # In case of reftest, one test calls the driver's run_test() twice. + # We should not add a reference html used by reftests to tests unless include_reference_html parameter + # is explicitly given. + if include_reference_html or not is_reference_html_file(test_input.filename): + self._current_test_batch.append(test_name) return TestDriver.run_test(self, test_input) class RecordingTestPort(TestPort): @@ -190,13 +197,13 @@ class MainTest(unittest.TestCase): def test_child_process_1(self): (res, buildbot_output, regular_output, user) = logging_run( - ['--print', 'config', '--child-processes', '1']) + ['--print', 'config', '--worker-model', 'threads', '--child-processes', '1']) self.assertTrue('Running one DumpRenderTree\n' in regular_output.get()) def test_child_processes_2(self): (res, buildbot_output, regular_output, user) = logging_run( - ['--print', 'config', '--child-processes', '2']) + ['--print', 'config', '--worker-model', 'threads', '--child-processes', '2']) self.assertTrue('Running 2 DumpRenderTrees in parallel\n' in regular_output.get()) @@ -352,7 +359,12 @@ class MainTest(unittest.TestCase): # Run tests including the unexpected failures. self._url_opened = None res, out, err, user = logging_run(tests_included=True) - self.assertEqual(res, 3) + + # 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 = 5 + + self.assertEqual(res, unexpected_tests_count) self.assertFalse(out.empty()) self.assertFalse(err.empty()) self.assertEqual(user.opened_urls, ['/tmp/layout-test-results/results.html']) @@ -458,6 +470,32 @@ class MainTest(unittest.TestCase): tests_included=True) self.assertEqual(user.opened_urls, ['/tmp/foo/results.html']) + # These next tests test that we run the tests in ascending alphabetical + # order per directory. HTTP tests are sharded separately from other tests, + # so we have to test both. + def assert_run_order(self, worker_model, child_processes='1'): + tests_run = get_tests_run(['--worker-model', worker_model, + '--child-processes', child_processes, 'passes'], + tests_included=True, flatten_batches=True) + self.assertEquals(tests_run, sorted(tests_run)) + + tests_run = get_tests_run(['--worker-model', worker_model, + '--child-processes', child_processes, 'http/tests/passes'], + tests_included=True, flatten_batches=True) + self.assertEquals(tests_run, sorted(tests_run)) + + def test_run_order__inline(self): + self.assert_run_order('inline') + + def test_run_order__old_inline(self): + self.assert_run_order('old-inline') + + def test_run_order__threads(self): + self.assert_run_order('old-inline', child_processes='2') + + def test_run_order__old_threads(self): + self.assert_run_order('old-threads', child_processes='2') + def test_tolerance(self): class ImageDiffTestPort(TestPort): def diff_image(self, expected_contents, actual_contents, @@ -487,11 +525,11 @@ class MainTest(unittest.TestCase): def test_worker_model__inline(self): self.assertTrue(passing_run(['--worker-model', 'inline'])) - def test_worker_model__old_inline_with_child_processes(self): - res, out, err, user = logging_run(['--worker-model', 'old-inline', + def test_worker_model__inline_with_child_processes(self): + res, out, err, user = logging_run(['--worker-model', 'inline', '--child-processes', '2']) self.assertEqual(res, 0) - self.assertTrue('--worker-model=old-inline overrides --child-processes\n' in err.get()) + self.assertTrue('--worker-model=inline overrides --child-processes\n' in err.get()) def test_worker_model__old_inline(self): self.assertTrue(passing_run(['--worker-model', 'old-inline'])) @@ -516,6 +554,25 @@ class MainTest(unittest.TestCase): self.assertRaises(ValueError, logging_run, ['--worker-model', 'unknown']) + def test_reftest_run(self): + tests_run = get_tests_run(['passes/reftest.html'], tests_included=True, flatten_batches=True) + self.assertEquals(['passes/reftest.html'], tests_run) + + def test_reftest_expected_html_should_be_ignored(self): + tests_run = get_tests_run(['passes/reftest-expected.html'], tests_included=True, flatten_batches=True) + self.assertEquals([], tests_run) + + def test_reftest_driver_should_run_expected_html(self): + tests_run = get_tests_run(['passes/reftest.html'], tests_included=True, flatten_batches=True, + include_reference_html=True) + self.assertEquals(['passes/reftest.html', 'passes/reftest-expected.html'], tests_run) + + def test_reftest_driver_should_run_expected_mismatch_html(self): + tests_run = get_tests_run(['passes/mismatch.html'], tests_included=True, flatten_batches=True, + include_reference_html=True) + self.assertEquals(['passes/mismatch.html', 'passes/mismatch-expected-mismatch.html'], tests_run) + + MainTest = skip_if(MainTest, sys.platform == 'cygwin' and compare_version(sys, '2.6')[0] < 0, 'new-run-webkit-tests tests hang on Cygwin Python 2.5.2') diff --git a/Tools/Scripts/webkitpy/style/checker.py b/Tools/Scripts/webkitpy/style/checker.py index 975432b..48abcf9 100644 --- a/Tools/Scripts/webkitpy/style/checker.py +++ b/Tools/Scripts/webkitpy/style/checker.py @@ -36,6 +36,7 @@ import sys from checkers.common import categories as CommonCategories from checkers.common import CarriageReturnChecker +from checkers.changelog import ChangeLogChecker from checkers.cpp import CppChecker from checkers.python import PythonChecker from checkers.test_expectations import TestExpectationsChecker @@ -180,6 +181,7 @@ _PATH_RULES_SPECIFIER = [ # struct members. Also, we allow unnecessary parameter names in # WebKit2 APIs because we're matching CF's header style. "Source/WebKit2/UIProcess/API/C/", + "Source/WebKit2/Shared/API/c/", "Source/WebKit2/WebProcess/InjectedBundle/API/c/"], ["-readability/naming", "-readability/parameter_name", @@ -419,10 +421,11 @@ class FileType: NONE = 0 # FileType.NONE evaluates to False. # Alphabetize remaining types - CPP = 1 - PYTHON = 2 - TEXT = 3 - XML = 4 + CHANGELOG = 1 + CPP = 2 + PYTHON = 3 + TEXT = 4 + XML = 5 class CheckerDispatcher(object): @@ -481,8 +484,9 @@ class CheckerDispatcher(object): return FileType.PYTHON elif file_extension in _XML_FILE_EXTENSIONS: return FileType.XML - elif (os.path.basename(file_path).startswith('ChangeLog') or - (not file_extension and os.path.join("Tools", "Scripts") in file_path) or + elif os.path.basename(file_path).startswith('ChangeLog'): + return FileType.CHANGELOG + elif ((not file_extension and os.path.join("Tools", "Scripts") in file_path) or file_extension in _TEXT_FILE_EXTENSIONS): return FileType.TEXT else: @@ -493,6 +497,11 @@ class CheckerDispatcher(object): """Instantiate and return a style checker based on file type.""" if file_type == FileType.NONE: checker = None + elif file_type == FileType.CHANGELOG: + should_line_be_checked = None + if handle_style_error: + should_line_be_checked = handle_style_error.should_line_be_checked + checker = ChangeLogChecker(file_path, handle_style_error, should_line_be_checked) elif file_type == FileType.CPP: file_extension = self._file_extension(file_path) checker = CppChecker(file_path, file_extension, diff --git a/Tools/Scripts/webkitpy/style/checker_unittest.py b/Tools/Scripts/webkitpy/style/checker_unittest.py index a796e0b..144d40a 100755 --- a/Tools/Scripts/webkitpy/style/checker_unittest.py +++ b/Tools/Scripts/webkitpy/style/checker_unittest.py @@ -52,6 +52,7 @@ from checker import CheckerDispatcher from checker import ProcessorBase from checker import StyleProcessor from checker import StyleProcessorConfiguration +from checkers.changelog import ChangeLogChecker from checkers.cpp import CppChecker from checkers.python import PythonChecker from checkers.text import TextChecker @@ -368,12 +369,10 @@ class CheckerDispatcherDispatchTest(unittest.TestCase): """Tests dispatch() method of CheckerDispatcher class.""" - def mock_handle_style_error(self): - pass - def dispatch(self, file_path): """Call dispatch() with the given file path.""" dispatcher = CheckerDispatcher() + self.mock_handle_style_error = DefaultStyleErrorHandler('', None, None, []) checker = dispatcher.dispatch(file_path, self.mock_handle_style_error, min_confidence=3) @@ -395,6 +394,10 @@ class CheckerDispatcherDispatchTest(unittest.TestCase): "got_class": got_class, "expected_class": expected_class}) + def assert_checker_changelog(self, file_path): + """Assert that the dispatched checker is a ChangeLogChecker.""" + self.assert_checker(file_path, ChangeLogChecker) + def assert_checker_cpp(self, file_path): """Assert that the dispatched checker is a CppChecker.""" self.assert_checker(file_path, CppChecker) @@ -411,6 +414,25 @@ class CheckerDispatcherDispatchTest(unittest.TestCase): """Assert that the dispatched checker is a XMLChecker.""" self.assert_checker(file_path, XMLChecker) + def test_changelog_paths(self): + """Test paths that should be checked as ChangeLog.""" + paths = [ + "ChangeLog", + "ChangeLog-2009-06-16", + os.path.join("Source", "WebCore", "ChangeLog"), + ] + + for path in paths: + self.assert_checker_changelog(path) + + # Check checker attributes on a typical input. + file_path = "ChangeLog" + self.assert_checker_changelog(file_path) + checker = self.dispatch(file_path) + self.assertEquals(checker.file_path, file_path) + self.assertEquals(checker.handle_style_error, + self.mock_handle_style_error) + def test_cpp_paths(self): """Test paths that should be checked as C++.""" paths = [ @@ -465,8 +487,6 @@ class CheckerDispatcherDispatchTest(unittest.TestCase): def test_text_paths(self): """Test paths that should be checked as text.""" paths = [ - "ChangeLog", - "ChangeLog-2009-06-16", "foo.ac", "foo.cc", "foo.cgi", @@ -491,7 +511,6 @@ class CheckerDispatcherDispatchTest(unittest.TestCase): "foo.wm", "foo.xhtml", "foo.y", - os.path.join("Source", "WebCore", "ChangeLog"), os.path.join("Source", "WebCore", "inspector", "front-end", "inspector.js"), os.path.join("Tools", "Scripts", "check-webkit-style"), ] diff --git a/Tools/Scripts/webkitpy/style/checkers/changelog.py b/Tools/Scripts/webkitpy/style/checkers/changelog.py new file mode 100644 index 0000000..75754fa --- /dev/null +++ b/Tools/Scripts/webkitpy/style/checkers/changelog.py @@ -0,0 +1,74 @@ +#!/usr/bin/env python +# +# Copyright (C) 2011 Patrick Gansterer <paroga@paroga.com> +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""Checks WebKit style for ChangeLog files.""" + +import re +from common import TabChecker +from webkitpy.common.net.bugzilla import parse_bug_id_from_changelog + + +class ChangeLogChecker(object): + + """Processes text lines for checking style.""" + + def __init__(self, file_path, handle_style_error, should_line_be_checked): + self.file_path = file_path + self.handle_style_error = handle_style_error + self.should_line_be_checked = should_line_be_checked + self._tab_checker = TabChecker(file_path, handle_style_error) + + def check_entry(self, first_line_checked, entry_lines): + if not entry_lines: + return + for line in entry_lines: + if parse_bug_id_from_changelog(line): + break + if re.search("Unreviewed", line, re.IGNORECASE): + break + if re.search("build", line, re.IGNORECASE) and re.search("fix", line, re.IGNORECASE): + break + else: + self.handle_style_error(first_line_checked, + "changelog/bugnumber", 5, + "ChangeLog entry has no bug number") + + def check(self, lines): + self._tab_checker.check(lines) + first_line_checked = 0 + entry_lines = [] + + for line_index, line in enumerate(lines): + if not self.should_line_be_checked(line_index + 1): + # If we transitioned from finding changed lines to + # unchanged lines, then we are done. + if first_line_checked: + break + continue + if not first_line_checked: + first_line_checked = line_index + 1 + entry_lines.append(line) + + self.check_entry(first_line_checked, entry_lines) diff --git a/Tools/Scripts/webkitpy/style/checkers/changelog_unittest.py b/Tools/Scripts/webkitpy/style/checkers/changelog_unittest.py new file mode 100644 index 0000000..02296d3 --- /dev/null +++ b/Tools/Scripts/webkitpy/style/checkers/changelog_unittest.py @@ -0,0 +1,155 @@ +#!/usr/bin/env python +# +# Copyright (C) 2010 Apple Inc. All rights reserved. +# Copyright (C) 2011 Patrick Gansterer <paroga@paroga.com> +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED +# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR +# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""Unit test for changelog.py.""" + +import changelog +import unittest + + +class ChangeLogCheckerTest(unittest.TestCase): + """Tests ChangeLogChecker class.""" + + def assert_no_error(self, lines_to_check, changelog_data): + def handle_style_error(line_number, category, confidence, message): + self.fail('Unexpected error: %d %s %d %s for\n%s' % (line_number, category, confidence, message, changelog_data)) + self.lines_to_check = set(lines_to_check) + checker = changelog.ChangeLogChecker('ChangeLog', handle_style_error, self.mock_should_line_be_checked) + checker.check(changelog_data.split('\n')) + + def assert_error(self, expected_line_number, lines_to_check, expected_category, changelog_data): + self.had_error = False + + def handle_style_error(line_number, category, confidence, message): + self.had_error = True + self.assertEquals(expected_line_number, line_number) + self.assertEquals(expected_category, category) + self.lines_to_check = set(lines_to_check) + checker = changelog.ChangeLogChecker('ChangeLog', handle_style_error, self.mock_should_line_be_checked) + checker.check(changelog_data.split('\n')) + self.assertTrue(self.had_error) + + def mock_handle_style_error(self): + pass + + def mock_should_line_be_checked(self, line_number): + return line_number in self.lines_to_check + + def test_init(self): + checker = changelog.ChangeLogChecker('ChangeLog', self.mock_handle_style_error, self.mock_should_line_be_checked) + self.assertEquals(checker.file_path, 'ChangeLog') + self.assertEquals(checker.handle_style_error, self.mock_handle_style_error) + self.assertEquals(checker.should_line_be_checked, self.mock_should_line_be_checked) + + def test_missing_bug_number(self): + self.assert_error(1, range(1, 20), 'changelog/bugnumber', + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + '\n' + ' Example bug\n') + self.assert_error(1, range(1, 20), 'changelog/bugnumber', + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + '\n' + ' Example bug\n' + ' http://bugs.webkit.org/show_bug.cgi?id=\n') + self.assert_error(1, range(1, 20), 'changelog/bugnumber', + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + '\n' + ' Example bug\n' + ' https://bugs.webkit.org/show_bug.cgi?id=\n') + self.assert_error(1, range(1, 20), 'changelog/bugnumber', + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + '\n' + ' Example bug\n' + ' http://webkit.org/b/\n') + self.assert_error(1, range(1, 20), 'changelog/bugnumber', + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + '\n' + ' Example bug' + '\n' + ' http://trac.webkit.org/changeset/12345\n') + self.assert_error(2, range(2, 5), 'changelog/bugnumber', + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + ' Example bug\n' + ' https://bugs.webkit.org/show_bug.cgi\n' + '\n' + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + ' Another change\n') + self.assert_error(2, range(2, 6), 'changelog/bugnumber', + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + ' Example bug\n' + ' More text about bug.\n' + '\n' + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + '\n' + ' No bug in this change.\n') + + def test_no_error(self): + self.assert_no_error([], + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + '\n' + ' Example ChangeLog entry out of range\n' + ' http://example.com/\n') + self.assert_no_error([], + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + '\n' + ' Example bug\n' + ' http://bugs.webkit.org/show_bug.cgi?id=12345\n') + self.assert_no_error(range(1, 20), + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + '\n' + ' Example bug\n' + ' http://bugs.webkit.org/show_bug.cgi?id=12345\n') + self.assert_no_error(range(1, 20), + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + '\n' + ' Example bug\n' + ' https://bugs.webkit.org/show_bug.cgi?id=12345\n') + self.assert_no_error(range(1, 20), + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + '\n' + ' Example bug\n' + ' http://webkit.org/b/12345\n') + self.assert_no_error(range(1, 20), + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + '\n' + ' Unreview build fix for r12345.\n') + self.assert_no_error(range(1, 20), + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + '\n' + ' Fix build after a bad change.\n') + self.assert_no_error(range(1, 20), + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + '\n' + ' Fix example port build.\n') + self.assert_no_error(range(2, 6), + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + ' Example bug\n' + ' https://bugs.webkit.org/show_bug.cgi?id=12345\n' + '\n' + '2011-01-01 Patrick Gansterer <paroga@paroga.com>\n' + ' No bug here!\n') + +if __name__ == '__main__': + unittest.main() diff --git a/Tools/Scripts/webkitpy/style/error_handlers.py b/Tools/Scripts/webkitpy/style/error_handlers.py index 0bede24..5d8b041 100644 --- a/Tools/Scripts/webkitpy/style/error_handlers.py +++ b/Tools/Scripts/webkitpy/style/error_handlers.py @@ -123,16 +123,18 @@ class DefaultStyleErrorHandler(object): return None return self._configuration.max_reports_per_category[category] + def should_line_be_checked(self, line_number): + "Returns if a particular line should be checked" + # Was the line that was modified? + return self._line_numbers is None or line_number in self._line_numbers + def __call__(self, line_number, category, confidence, message): """Handle the occurrence of a style error. See the docstring of this module for more information. """ - if (self._line_numbers is not None and - line_number not in self._line_numbers): - # Then the error occurred in a line that was not modified, so - # the error is not reportable. + if not self.should_line_be_checked(line_number): return if not self._configuration.is_reportable(category=category, diff --git a/Tools/Scripts/webkitpy/test/cat.py b/Tools/Scripts/webkitpy/test/cat.py index ae1e143..ac56d1c 100644 --- a/Tools/Scripts/webkitpy/test/cat.py +++ b/Tools/Scripts/webkitpy/test/cat.py @@ -30,7 +30,7 @@ from webkitpy.common.system import fileutils def command_arguments(*args): - return ['python', __file__] + list(args) + return [sys.executable, __file__] + list(args) def main(): diff --git a/Tools/Scripts/webkitpy/test/echo.py b/Tools/Scripts/webkitpy/test/echo.py index f7468f7..5d4d8e2 100644 --- a/Tools/Scripts/webkitpy/test/echo.py +++ b/Tools/Scripts/webkitpy/test/echo.py @@ -30,7 +30,7 @@ from webkitpy.common.system import fileutils def command_arguments(*args): - return ['python', __file__] + list(args) + return [sys.executable, __file__] + list(args) def main(args=None): diff --git a/Tools/Scripts/webkitpy/thirdparty/__init__.py b/Tools/Scripts/webkitpy/thirdparty/__init__.py index c2249c2..9728492 100644 --- a/Tools/Scripts/webkitpy/thirdparty/__init__.py +++ b/Tools/Scripts/webkitpy/thirdparty/__init__.py @@ -70,7 +70,7 @@ installer.install(url="http://pypi.python.org/packages/source/C/ClientForm/Clien # a new AutoInstaller instance that does not append to the search path. installer = AutoInstaller(target_dir=autoinstalled_dir) -installer.install(url="http://pypi.python.org/packages/source/m/mechanize/mechanize-0.1.11.zip", +installer.install(url="http://pypi.python.org/packages/source/m/mechanize/mechanize-0.2.4.zip", url_subpath="mechanize") installer.install(url="http://pypi.python.org/packages/source/p/pep8/pep8-0.5.0.tar.gz#md5=512a818af9979290cd619cce8e9c2e2b", url_subpath="pep8-0.5.0/pep8.py") diff --git a/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py b/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py index b22138d..c5d9001 100644 --- a/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py +++ b/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py @@ -178,8 +178,6 @@ class CommitQueueTask(object): self._delegate.report_flaky_tests(self._patch, flaky_test_results, results_archive) def _test_patch(self): - if self._patch.is_rollout(): - return True if self._test(): return True @@ -220,12 +218,13 @@ class CommitQueueTask(object): return False if not self._apply(): return self.report_failure() - if not self._build(): - if not self._build_without_patch(): + if not self._patch.is_rollout(): + if not self._build(): + if not self._build_without_patch(): + return False + return self.report_failure() + if not self._test_patch(): return False - return self.report_failure() - if not self._test_patch(): - return False # Make sure the patch is still valid before landing (e.g., make sure # no one has set commit-queue- since we started working on the patch.) if not self._validate(): diff --git a/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py b/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py index 8f5c9e6..e2fb09f 100644 --- a/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py +++ b/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py @@ -293,8 +293,6 @@ MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'update'] MOCK: update_status: commit-queue Updated working directory MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'apply-attachment', '--no-update', '--non-interactive', 106] MOCK: update_status: commit-queue Applied patch -MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build', '--no-clean', '--no-update', '--build-style=both'] -MOCK: update_status: commit-queue Built patch MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--ignore-builders', '--non-interactive', '--parent-command=commit-queue', 106] MOCK: update_status: commit-queue Landed patch MOCK: update_status: commit-queue Pass diff --git a/Tools/Scripts/webkitpy/tool/commands/upload.py b/Tools/Scripts/webkitpy/tool/commands/upload.py index e455b18..80715a7 100644 --- a/Tools/Scripts/webkitpy/tool/commands/upload.py +++ b/Tools/Scripts/webkitpy/tool/commands/upload.py @@ -37,7 +37,7 @@ from optparse import make_option from webkitpy.tool import steps from webkitpy.common.config.committers import CommitterList -from webkitpy.common.net.bugzilla import parse_bug_id +from webkitpy.common.net.bugzilla import parse_bug_id_from_changelog from webkitpy.common.system.deprecated_logging import error, log from webkitpy.common.system.user import User from webkitpy.thirdparty.mock import Mock @@ -173,6 +173,21 @@ class ObsoleteAttachments(AbstractSequencedCommand): return { "bug_id" : args[0] } +class AttachToBug(AbstractSequencedCommand): + name = "attach-to-bug" + help_text = "Attach the the file to the bug" + argument_names = "BUGID FILEPATH" + steps = [ + steps.AttachToBug, + ] + + def _prepare_state(self, options, args, tool): + state = {} + state["bug_id"] = args[0] + state["filepath"] = args[1] + return state + + class AbstractPatchUploadingCommand(AbstractSequencedCommand): def _bug_id(self, options, args, tool, state): # Perfer a bug id passed as an argument over a bug url in the diff (i.e. ChangeLogs). @@ -311,7 +326,7 @@ class PostCommits(AbstractDeclarativeCommand): commit_message = tool.scm().commit_message_for_local_commit(commit_id) # Prefer --bug-id=, then a bug url in the commit message, then a bug url in the entire commit diff (i.e. ChangeLogs). - bug_id = options.bug_id or parse_bug_id(commit_message.message()) or parse_bug_id(tool.scm().create_patch(git_commit=commit_id)) + bug_id = options.bug_id or parse_bug_id_from_changelog(commit_message.message()) or parse_bug_id_from_changelog(tool.scm().create_patch(git_commit=commit_id)) if not bug_id: log("Skipping %s: No bug id found in commit or specified with --bug-id." % commit_id) continue @@ -351,7 +366,7 @@ class MarkBugFixed(AbstractDeclarativeCommand): commit_log = self._fetch_commit_log(tool, svn_revision) if not bug_id: - bug_id = parse_bug_id(commit_log) + bug_id = parse_bug_id_from_changelog(commit_log) if not svn_revision: match = re.search("^r(?P<svn_revision>\d+) \|", commit_log, re.MULTILINE) diff --git a/Tools/Scripts/webkitpy/tool/commands/upload_unittest.py b/Tools/Scripts/webkitpy/tool/commands/upload_unittest.py index b5f5ae9..4313df9 100644 --- a/Tools/Scripts/webkitpy/tool/commands/upload_unittest.py +++ b/Tools/Scripts/webkitpy/tool/commands/upload_unittest.py @@ -68,6 +68,25 @@ MOCK: user.open_url: http://example.com/42 """ self.assert_execute_outputs(Post(), [42], options=options, expected_stderr=expected_stderr) + def test_attach_to_bug(self): + options = MockOptions() + options.comment = "extra comment" + options.description = "file description" + expected_stderr = """MOCK add_attachment_to_bug: bug_id=42, description=file description filename=None +-- Begin comment -- +extra comment +-- End comment -- +""" + self.assert_execute_outputs(AttachToBug(), [42, "path/to/file.txt", "file description"], options=options, expected_stderr=expected_stderr) + + def test_attach_to_bug_no_description_or_comment(self): + options = MockOptions() + options.comment = None + options.description = None + expected_stderr = """MOCK add_attachment_to_bug: bug_id=42, description=file.txt filename=None +""" + self.assert_execute_outputs(AttachToBug(), [42, "path/to/file.txt"], options=options, expected_stderr=expected_stderr) + def test_land_safely(self): expected_stderr = "Obsoleting 2 old patches on bug 42\nMOCK add_patch_to_bug: bug_id=42, description=Patch for landing, mark_for_review=False, mark_for_commit_queue=False, mark_for_landing=True\n" self.assert_execute_outputs(LandSafely(), [42], expected_stderr=expected_stderr) diff --git a/Tools/Scripts/webkitpy/tool/steps/__init__.py b/Tools/Scripts/webkitpy/tool/steps/__init__.py index d5d7bb4..a746602 100644 --- a/Tools/Scripts/webkitpy/tool/steps/__init__.py +++ b/Tools/Scripts/webkitpy/tool/steps/__init__.py @@ -29,6 +29,7 @@ # FIXME: Is this the right way to do this? from webkitpy.tool.steps.applypatch import ApplyPatch from webkitpy.tool.steps.applypatchwithlocalcommit import ApplyPatchWithLocalCommit +from webkitpy.tool.steps.attachtobug import AttachToBug from webkitpy.tool.steps.build import Build from webkitpy.tool.steps.checkstyle import CheckStyle from webkitpy.tool.steps.cleanworkingdirectory import CleanWorkingDirectory diff --git a/Tools/Scripts/webkitpy/tool/steps/abstractstep.py b/Tools/Scripts/webkitpy/tool/steps/abstractstep.py index 1c93d5b..2ba4291 100644 --- a/Tools/Scripts/webkitpy/tool/steps/abstractstep.py +++ b/Tools/Scripts/webkitpy/tool/steps/abstractstep.py @@ -40,7 +40,7 @@ class AbstractStep(object): # FIXME: This should use tool.port() def _run_script(self, script_name, args=None, quiet=False, port=WebKitPort): log("Running %s" % script_name) - command = [port.script_path(script_name)] + command = port.script_shell_command(script_name) if args: command.extend(args) self._tool.executive.run_and_throw_if_fail(command, quiet) diff --git a/Tools/Scripts/webkitpy/tool/steps/attachtobug.py b/Tools/Scripts/webkitpy/tool/steps/attachtobug.py new file mode 100644 index 0000000..4885fcb --- /dev/null +++ b/Tools/Scripts/webkitpy/tool/steps/attachtobug.py @@ -0,0 +1,51 @@ +# Copyright (C) 2011 Google Inc. All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +import os + +from webkitpy.tool.steps.abstractstep import AbstractStep +from webkitpy.tool.steps.options import Options + + +class AttachToBug(AbstractStep): + @classmethod + def options(cls): + return AbstractStep.options() + [ + Options.comment, + Options.description, + ] + + def run(self, state): + filepath = state["filepath"] + bug_id = state["bug_id"] + description = self._options.description or filepath.split(os.sep)[-1] + comment_text = self._options.comment + + # add_attachment_to_bug fills in the filename from the file path. + filename = None + self._tool.bugs.add_attachment_to_bug(bug_id, filepath, description, filename, comment_text) diff --git a/Tools/Scripts/webkitpy/tool/steps/commit.py b/Tools/Scripts/webkitpy/tool/steps/commit.py index 859acbf..5dc4efb 100644 --- a/Tools/Scripts/webkitpy/tool/steps/commit.py +++ b/Tools/Scripts/webkitpy/tool/steps/commit.py @@ -58,7 +58,7 @@ class Commit(AbstractStep): try: scm = self._tool.scm() - commit_text = scm.commit_with_message(self._commit_message, git_commit=self._options.git_commit, username=username, force_squash=force_squash) + commit_text = scm.commit_with_message(self._commit_message, git_commit=self._options.git_commit, username=username, force_squash=force_squash, changed_files=self._changed_files(state)) svn_revision = scm.svn_revision_from_commit_text(commit_text) log("Committed r%s: <%s>" % (svn_revision, urls.view_revision_url(svn_revision))) self._state["commit_text"] = commit_text diff --git a/Tools/Scripts/webkitpy/tool/steps/options.py b/Tools/Scripts/webkitpy/tool/steps/options.py index 5b8baf0..3bba3e2 100644 --- a/Tools/Scripts/webkitpy/tool/steps/options.py +++ b/Tools/Scripts/webkitpy/tool/steps/options.py @@ -40,7 +40,7 @@ class Options(object): comment = make_option("--comment", action="store", type="string", dest="comment", help="Comment to post to bug.") component = make_option("--component", action="store", type="string", dest="component", help="Component for the new bug.") confirm = make_option("--no-confirm", action="store_false", dest="confirm", default=True, help="Skip confirmation steps.") - description = make_option("-m", "--description", action="store", type="string", dest="description", help="Description string for the attachment (default: \"patch\")") + description = make_option("-m", "--description", action="store", type="string", dest="description", help="Description string for the attachment") email = make_option("--email", action="store", type="string", dest="email", help="Email address to use in ChangeLogs.") force_clean = make_option("--force-clean", action="store_true", dest="force_clean", default=False, help="Clean working directory before applying patches (removes local changes and commits)") force_patch = make_option("--force-patch", action="store_true", dest="force_patch", default=False, help="Forcefully applies the patch, continuing past errors.") diff --git a/Tools/Scripts/webkitpy/tool/steps/preparechangelog.py b/Tools/Scripts/webkitpy/tool/steps/preparechangelog.py index 17e996c..4be40ca 100644 --- a/Tools/Scripts/webkitpy/tool/steps/preparechangelog.py +++ b/Tools/Scripts/webkitpy/tool/steps/preparechangelog.py @@ -61,7 +61,7 @@ class PrepareChangeLog(AbstractStep): self._ensure_bug_url(state) return os.chdir(self._tool.scm().checkout_root) - args = [self._tool.port().script_path("prepare-ChangeLog")] + args = self._tool.port().script_shell_command("prepare-ChangeLog") if state.get("bug_id"): args.append("--bug=%s" % state["bug_id"]) args.append("--description=%s" % self._tool.bugs.fetch_bug(state["bug_id"]).title()) |