diff options
Diffstat (limited to 'WebKitTools/Scripts')
69 files changed, 3059 insertions, 1093 deletions
diff --git a/WebKitTools/Scripts/build-webkit b/WebKitTools/Scripts/build-webkit index da336fe..d9534f8 100755 --- a/WebKitTools/Scripts/build-webkit +++ b/WebKitTools/Scripts/build-webkit @@ -87,7 +87,6 @@ my ( $notificationsSupport, $offlineWebApplicationSupport, $progressTagSupport, - $rubySupport, $sharedWorkersSupport, $svgSupport, $svgAnimationSupport, @@ -198,9 +197,6 @@ my @features = ( { option => "progress-tag", desc => "Progress Tag support", define => "ENABLE_PROGRESS_TAG", default => 1, value => \$progressTagSupport }, - { option => "ruby", desc => "Toggle HTML5 Ruby support", - define => "ENABLE_RUBY", default => 1, value => \$rubySupport }, - { option => "system-malloc", desc => "Toggle system allocator instead of TCmalloc", define => "USE_SYSTEM_MALLOC", default => 0, value => \$systemMallocSupport }, diff --git a/WebKitTools/Scripts/do-file-rename b/WebKitTools/Scripts/do-file-rename index b81b9dc..9c4c898 100755 --- a/WebKitTools/Scripts/do-file-rename +++ b/WebKitTools/Scripts/do-file-rename @@ -47,6 +47,7 @@ find(\&wanted, "JavaScriptCore"); find(\&wanted, "JavaScriptGlue"); find(\&wanted, "WebCore"); find(\&wanted, "WebKit"); +find(\&wanted, "WebKit2"); sub wanted { diff --git a/WebKitTools/Scripts/old-run-webkit-tests b/WebKitTools/Scripts/old-run-webkit-tests index 2cef18b..eeaaab3 100755 --- a/WebKitTools/Scripts/old-run-webkit-tests +++ b/WebKitTools/Scripts/old-run-webkit-tests @@ -917,8 +917,7 @@ for my $test (@tests) { testCrashedOrTimedOut($test, $base, 1, $actual, $error); } elsif (!defined $expected) { if ($verbose) { - print "new " . ($resetResults ? "result" : "test") ."\n"; - $atLineStart = 1; + print "new " . ($resetResults ? "result" : "test"); } $result = "new"; @@ -936,11 +935,13 @@ for my $test (@tests) { print "$test -> "; } my $resultsDir = catdir($expectedDir, dirname($base)); + if (!$verbose) { + print "new"; + } if ($generateNewResults) { - print "new (results generated in $resultsDir)\n"; - } else { - print "new\n"; + print " (results generated in $resultsDir)"; } + print "\n" unless $atLineStart; $atLineStart = 1; } } elsif ($actual eq $expected && $diffResult eq "passed") { @@ -1420,6 +1421,10 @@ sub openDumpTool() { return if $isDumpToolOpen; + if ($verbose && $testsPerDumpTool != 1) { + print "| Opening DumpTool |\n"; + } + my %CLEAN_ENV; # Generic environment variables @@ -1502,6 +1507,10 @@ sub closeDumpTool() { return if !$isDumpToolOpen; + if ($verbose && $testsPerDumpTool != 1) { + print "| Closing DumpTool |\n"; + } + close IN; close OUT; waitpid $dumpToolPID, 0; diff --git a/WebKitTools/Scripts/print-vse-failure-logs b/WebKitTools/Scripts/print-vse-failure-logs index cec806e..7580465 100755 --- a/WebKitTools/Scripts/print-vse-failure-logs +++ b/WebKitTools/Scripts/print-vse-failure-logs @@ -33,15 +33,14 @@ from __future__ import with_statement import codecs -import os.path import os +import re +from webkitpy.common.checkout import scm from webkitpy.common.system.executive import Executive +from webkitpy.thirdparty import BeautifulSoup -# This is just enough to make the win-ews bot useable. Others who -# actually use Windows should feel encouraged to improve this class, -# including just printing the text instead of the raw html. class PrintVisualStudioExpressLogs(object): def __init__(self): self._executive = Executive() @@ -55,25 +54,59 @@ class PrintVisualStudioExpressLogs(object): build_log_paths.append(file_path) return build_log_paths + def _build_order(self): + """Returns a list of project names in the order in which they are built.""" + script_path = os.path.join(self._scripts_directory(), "print-msvc-project-dependencies") + sln_path = os.path.join(scm.find_checkout_root(), "WebKit", "win", "WebKit.vcproj", "WebKit.sln") + lines = self._executive.run_command([script_path, sln_path]).splitlines() + order = [line.strip() for line in lines if line.find("Folder") == -1] + order.reverse() + return order + + def _sort_buildlogs(self, log_paths): + build_order = self._build_order() + def sort_key(log_path): + project_name = os.path.basename(os.path.dirname(os.path.dirname(log_path))) + try: + index = build_order.index(project_name) + except ValueError: + # If the project isn't in the list, sort it after all items that + # are in the list. + index = len(build_order) + # Sort first by build order, then by project name + return (index, project_name) + return sorted(log_paths, key=sort_key) + def _obj_directory(self): - scripts_directory = os.path.dirname(__file__) - build_directory_script_path = os.path.join(scripts_directory, "webkit-build-directory") + build_directory_script_path = os.path.join(self._scripts_directory(), "webkit-build-directory") # FIXME: ports/webkit.py should provide the build directory in a nice API. # NOTE: The windows VSE build does not seem to use different directories # for Debug and Release. build_directory = self._executive.run_command([build_directory_script_path, "--top-level"]).rstrip() return os.path.join(build_directory, "obj") + def _scripts_directory(self): + return os.path.dirname(__file__) + + def _relevant_text(self, log): + soup = BeautifulSoup.BeautifulSoup(log) + # The Output Window table is where the useful output starts in the build log. + output_window_table = soup.find(text=re.compile("Output Window")).findParent("table") + result = [] + for table in [output_window_table] + output_window_table.findNextSiblings("table"): + result.extend([text.replace(" ", "") for text in table.findAll(text=True)]) + result.append("\n") + return "".join(result) + def main(self): - obj_directory = self._obj_directory() - build_log_paths = self._find_buildlogs(obj_directory) + build_log_paths = self._sort_buildlogs(self._find_buildlogs(self._obj_directory())) print "Found %s Visual Studio Express Build Logs:\n%s" % (len(build_log_paths), "\n".join(build_log_paths)) for build_log_path in build_log_paths: print "%s:\n" % build_log_path with codecs.open(build_log_path, "r", "utf-16") as build_log: - print build_log.read() + print self._relevant_text(build_log) if __name__ == '__main__': diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/scm.py b/WebKitTools/Scripts/webkitpy/common/checkout/scm.py index 8aadcb8..11e82ac 100644 --- a/WebKitTools/Scripts/webkitpy/common/checkout/scm.py +++ b/WebKitTools/Scripts/webkitpy/common/checkout/scm.py @@ -700,8 +700,7 @@ class Git(SCM): """Returns a byte array (str()) representing the patch file. Patch files are effectively binary since they may contain files of multiple different encodings.""" - # FIXME: This should probably use cwd=self.checkout_root - return self.run(['git', 'diff', '--binary', "--no-ext-diff", "--full-index", "-M", self.merge_base(git_commit), "--"] + changed_files, decode_output=False) + return self.run(['git', 'diff', '--binary', "--no-ext-diff", "--full-index", "-M", self.merge_base(git_commit), "--"] + changed_files, decode_output=False, cwd=self.checkout_root) def _run_git_svn_find_rev(self, arg): # git svn find-rev always exits 0, even when the revision or commit is not found. diff --git a/WebKitTools/Scripts/webkitpy/common/config/build.py b/WebKitTools/Scripts/webkitpy/common/config/build.py new file mode 100644 index 0000000..c45f122 --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/common/config/build.py @@ -0,0 +1,138 @@ +# Copyright (C) 2010 Apple 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 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. + +"""Functions relating to building WebKit""" + +import re + + +def _should_file_trigger_build(target_platform, file): + # The directories and patterns lists below map directory names or + # regexp patterns to the bot platforms for which they should trigger a + # build. Mapping to the empty list means that no builds should be + # triggered on any platforms. Earlier directories/patterns take + # 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") + + directories = [ + # Directories that shouldn't trigger builds on any bots. + ("BugsSite", []), + ("PageLoadTests", []), + ("PlanetWebKit", []), + ("WebCore/manual-tests", []), + ("WebKitExamplePlugins", []), + ("WebKitSite", []), + ("android", []), + ("brew", []), + ("efl", []), + ("haiku", []), + ("iphone", []), + ("opengl", []), + ("opentype", []), + ("openvg", []), + ("wx", []), + ("wince", []), + + # Directories that should trigger builds on only some bots. + ("JavaScriptGlue", ["mac"]), + ("LayoutTests/platform/mac", ["mac", "win"]), + ("LayoutTests/platform/mac-snowleopard", ["mac-snowleopard", "win"]), + ("WebCore/image-decoders", ["chromium"]), + ("cairo", ["gtk", "wincairo"]), + ("cf", ["chromium-mac", "mac", "qt", "win"]), + ("chromium", ["chromium"]), + ("cocoa", ["chromium-mac", "mac"]), + ("curl", ["gtk", "wincairo"]), + ("gobject", ["gtk"]), + ("gpu", ["chromium", "mac"]), + ("gstreamer", ["gtk"]), + ("gtk", ["gtk"]), + ("mac", ["chromium-mac", "mac"]), + ("mac-leopard", ["mac-leopard"]), + ("mac-snowleopard", ["mac-snowleopard"]), + ("mac-wk2", ["mac-snowleopard", "win"]), + ("objc", ["mac"]), + ("qt", ["qt"]), + ("skia", ["chromium"]), + ("soup", ["gtk"]), + ("v8", ["chromium"]), + ("win", ["chromium-win", "win"]), + ] + patterns = [ + # Patterns that shouldn't trigger builds on any bots. + (r"(?:^|/)Makefile$", []), + (r"/ARM", []), + (r"/CMake.*", []), + (r"/ChangeLog.*$", []), + (r"/LICENSE[^/]+$", []), + (r"ARM(?:v7)?\.(?:cpp|h)$", []), + (r"MIPS\.(?:cpp|h)$", []), + (r"WinCE\.(?:cpp|h|mm)$", []), + (r"\.(?:bkl|mk)$", []), + + # Patterns that should trigger builds on only some bots. + (r"/GNUmakefile\.am$", ["gtk"]), + (r"/\w+Chromium\w*\.(?:cpp|h|mm)$", ["chromium"]), + (r"Mac\.(?:cpp|h|mm)$", ["mac"]), + (r"\.exp$", ["mac"]), + (r"\.gypi?", ["chromium"]), + (r"\.order$", ["mac"]), + (r"\.pr[io]$", ["qt"]), + (r"\.xcconfig$", ["mac"]), + (r"\.xcodeproj/", ["mac"]), + ] + + base_platform = target_platform.split("-")[0] + + # See if the file is in one of the known directories. + for directory, platforms in directories: + if re.search(r"(?:^|/)%s/" % directory, file): + return target_platform in platforms or base_platform in platforms + + # See if the file matches a known pattern. + for pattern, platforms in patterns: + if re.search(pattern, file): + return target_platform in platforms or base_platform in platforms + + # See if the file is a platform-specific test result. + match = re.match("LayoutTests/platform/(?P<platform>[^/]+)/", file) + if match: + # See if the file is a test result for this platform, our base + # platform, or one of our sub-platforms. + return match.group("platform") in (target_platform, base_platform) or match.group("platform").startswith("%s-" % target_platform) + + # The file isn't one we know about specifically, so we should assume we + # have to build. + return True + + +def should_build(target_platform, changed_files): + """Returns true if the changed files affect the given platform, and + thus a build should be performed. target_platform should be one of the + platforms used in the build.webkit.org master's config.json file.""" + return any(_should_file_trigger_build(target_platform, file) for file in changed_files) diff --git a/WebKitTools/Scripts/webkitpy/common/config/build_unittest.py b/WebKitTools/Scripts/webkitpy/common/config/build_unittest.py new file mode 100644 index 0000000..3e70ff0 --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/common/config/build_unittest.py @@ -0,0 +1,64 @@ +# Copyright (C) 2010 Apple 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 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. + +import unittest + +from webkitpy.common.config import build + + +class ShouldBuildTest(unittest.TestCase): + _should_build_tests = [ + (["BugsSite/foo", "WebCore/bar"], ["*"]), + (["BugsSite/foo"], []), + (["JavaScriptCore/JavaScriptCore.xcodeproj/foo"], ["mac-leopard", "mac-snowleopard"]), + (["JavaScriptGlue/foo", "WebCore/bar"], ["*"]), + (["JavaScriptGlue/foo"], ["mac-leopard", "mac-snowleopard"]), + (["LayoutTests/foo"], ["*"]), + (["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-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"]), + (["WebCore/mac/foo"], ["chromium-mac", "mac-leopard", "mac-snowleopard"]), + (["WebCore/win/foo"], ["chromium-win", "win"]), + (["WebCore/platform/graphics/gpu/foo"], ["mac-leopard", "mac-snowleopard"]), + (["WebCore/platform/wx/wxcode/win/foo"], []), + (["WebCore/rendering/RenderThemeMac.mm", "WebCore/rendering/RenderThemeMac.h"], ["mac-leopard", "mac-snowleopard"]), + (["WebCore/rendering/RenderThemeChromiumLinux.h"], ["chromium-linux"]), + (["WebCore/rendering/RenderThemeWinCE.h"], []), + ] + + 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"]: + 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))) + + +if __name__ == "__main__": + unittest.main() diff --git a/WebKitTools/Scripts/webkitpy/common/config/committers.py b/WebKitTools/Scripts/webkitpy/common/config/committers.py index 446c2b1..0967340 100644 --- a/WebKitTools/Scripts/webkitpy/common/config/committers.py +++ b/WebKitTools/Scripts/webkitpy/common/config/committers.py @@ -69,7 +69,7 @@ committers_unable_to_review = [ Committer("Albert J. Wong", "ajwong@chromium.org"), 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"), + Committer("Alexander Pavlov", "apavlov@chromium.org", "apavlov"), Committer("Andre Boule", "aboule@apple.com"), Committer("Andrei Popescu", "andreip@google.com", "andreip"), Committer("Andrew Wellington", ["andrew@webkit.org", "proton@wiretapped.net"], "proton"), diff --git a/WebKitTools/Scripts/webkitpy/common/config/committervalidator.py b/WebKitTools/Scripts/webkitpy/common/config/committervalidator.py new file mode 100644 index 0000000..b7b2990 --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/common/config/committervalidator.py @@ -0,0 +1,120 @@ +# Copyright (c) 2009 Google Inc. All rights reserved. +# Copyright (c) 2009 Apple Inc. All rights reserved. +# Copyright (c) 2010 Research In Motion Limited. 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.common.system.ospath import relpath +from webkitpy.common.config import committers + + +class CommitterValidator(object): + + def __init__(self, bugzilla): + self._bugzilla = bugzilla + + # _view_source_url belongs in some sort of webkit_config.py module. + def _view_source_url(self, local_path): + return "http://trac.webkit.org/browser/trunk/%s" % local_path + + def _checkout_root(self): + # FIXME: This is a hack, we would have this from scm.checkout_root + # if we had any way to get to an scm object here. + components = __file__.split(os.sep) + tools_index = components.index("WebKitTools") + return os.sep.join(components[:tools_index]) + + def _committers_py_path(self): + # extension can sometimes be .pyc, we always want .py + (path, extension) = os.path.splitext(committers.__file__) + # FIXME: When we're allowed to use python 2.6 we can use the real + # os.path.relpath + path = relpath(path, self._checkout_root()) + return ".".join([path, "py"]) + + def _flag_permission_rejection_message(self, setter_email, flag_name): + # Should come from some webkit_config.py + contribution_guidlines = "http://webkit.org/coding/contributing.html" + # This could be queried from the status_server. + queue_administrator = "eseidel@chromium.org" + # This could be queried from the tool. + queue_name = "commit-queue" + committers_list = self._committers_py_path() + message = "%s does not have %s permissions according to %s." % ( + setter_email, + flag_name, + self._view_source_url(committers_list)) + message += "\n\n- If you do not have %s rights please read %s for instructions on how to use bugzilla flags." % ( + flag_name, contribution_guidlines) + message += "\n\n- If you have %s rights please correct the error in %s by adding yourself to the file (no review needed). " % ( + flag_name, committers_list) + message += "The %s restarts itself every 2 hours. After restart the %s will correctly respect your %s rights." % ( + queue_name, queue_name, flag_name) + return message + + def _validate_setter_email(self, patch, result_key, rejection_function): + committer = getattr(patch, result_key)() + # If the flag is set, and we don't recognize the setter, reject the + # flag! + setter_email = patch._attachment_dictionary.get("%s_email" % result_key) + if setter_email and not committer: + rejection_function(patch.id(), + self._flag_permission_rejection_message(setter_email, + result_key)) + return False + return True + + def _reject_patch_if_flags_are_invalid(self, patch): + return (self._validate_setter_email( + patch, "reviewer", self.reject_patch_from_review_queue) + and self._validate_setter_email( + patch, "committer", self.reject_patch_from_commit_queue)) + + def patches_after_rejecting_invalid_commiters_and_reviewers(self, patches): + return [patch for patch in patches if self._reject_patch_if_flags_are_invalid(patch)] + + def reject_patch_from_commit_queue(self, + attachment_id, + additional_comment_text=None): + comment_text = "Rejecting patch %s from commit-queue." % attachment_id + self._bugzilla.set_flag_on_attachment(attachment_id, + "commit-queue", + "-", + comment_text, + additional_comment_text) + + def reject_patch_from_review_queue(self, + attachment_id, + additional_comment_text=None): + comment_text = "Rejecting patch %s from review queue." % attachment_id + self._bugzilla.set_flag_on_attachment(attachment_id, + 'review', + '-', + comment_text, + additional_comment_text) diff --git a/WebKitTools/Scripts/webkitpy/common/config/committervalidator_unittest.py b/WebKitTools/Scripts/webkitpy/common/config/committervalidator_unittest.py new file mode 100644 index 0000000..61fa3bf --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/common/config/committervalidator_unittest.py @@ -0,0 +1,43 @@ +# Copyright (C) 2010 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 unittest + +from .committervalidator import CommitterValidator + + +class CommitterValidatorTest(unittest.TestCase): + def test_flag_permission_rejection_message(self): + validator = CommitterValidator(bugzilla=None) + self.assertEqual(validator._committers_py_path(), "WebKitTools/Scripts/webkitpy/common/config/committers.py") + expected_messsage = """foo@foo.com does not have review permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py. + +- If you do not have review rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. + +- If you have review rights please correct the error in WebKitTools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your review rights.""" + self.assertEqual(validator._flag_permission_rejection_message("foo@foo.com", "review"), expected_messsage) diff --git a/WebKitTools/Scripts/webkitpy/common/net/bugzilla/__init__.py b/WebKitTools/Scripts/webkitpy/common/net/bugzilla/__init__.py new file mode 100644 index 0000000..cfaf3b1 --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/common/net/bugzilla/__init__.py @@ -0,0 +1,8 @@ +# Required for Python to search this directory for module files + +# We only export public API here. +# FIXME: parse_bug_id should not be a free function. +from .bugzilla import Bugzilla, parse_bug_id +# Unclear if Bug and Attachment need to be public classes. +from .bug import Bug +from .attachment import Attachment diff --git a/WebKitTools/Scripts/webkitpy/common/net/bugzilla/attachment.py b/WebKitTools/Scripts/webkitpy/common/net/bugzilla/attachment.py new file mode 100644 index 0000000..85761fe --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/common/net/bugzilla/attachment.py @@ -0,0 +1,114 @@ +# Copyright (c) 2009 Google Inc. All rights reserved. +# Copyright (c) 2009 Apple Inc. All rights reserved. +# Copyright (c) 2010 Research In Motion Limited. 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. + +from webkitpy.common.system.deprecated_logging import log + + +class Attachment(object): + + rollout_preamble = "ROLLOUT of r" + + def __init__(self, attachment_dictionary, bug): + self._attachment_dictionary = attachment_dictionary + self._bug = bug + self._reviewer = None + self._committer = None + + def _bugzilla(self): + return self._bug._bugzilla + + def id(self): + return int(self._attachment_dictionary.get("id")) + + def attacher_is_committer(self): + return self._bugzilla.committers.committer_by_email( + patch.attacher_email()) + + def attacher_email(self): + return self._attachment_dictionary.get("attacher_email") + + def bug(self): + return self._bug + + def bug_id(self): + return int(self._attachment_dictionary.get("bug_id")) + + def is_patch(self): + return not not self._attachment_dictionary.get("is_patch") + + def is_obsolete(self): + return not not self._attachment_dictionary.get("is_obsolete") + + def is_rollout(self): + return self.name().startswith(self.rollout_preamble) + + def name(self): + return self._attachment_dictionary.get("name") + + def attach_date(self): + return self._attachment_dictionary.get("attach_date") + + def review(self): + return self._attachment_dictionary.get("review") + + def commit_queue(self): + return self._attachment_dictionary.get("commit-queue") + + def url(self): + # FIXME: This should just return + # self._bugzilla().attachment_url_for_id(self.id()). scm_unittest.py + # depends on the current behavior. + return self._attachment_dictionary.get("url") + + def contents(self): + # FIXME: We shouldn't be grabbing at _bugzilla. + return self._bug._bugzilla.fetch_attachment_contents(self.id()) + + def _validate_flag_value(self, flag): + email = self._attachment_dictionary.get("%s_email" % flag) + if not email: + return None + committer = getattr(self._bugzilla().committers, + "%s_by_email" % flag)(email) + if committer: + return committer + log("Warning, attachment %s on bug %s has invalid %s (%s)" % ( + self._attachment_dictionary['id'], + self._attachment_dictionary['bug_id'], flag, email)) + + def reviewer(self): + if not self._reviewer: + self._reviewer = self._validate_flag_value("reviewer") + return self._reviewer + + def committer(self): + if not self._committer: + self._committer = self._validate_flag_value("committer") + return self._committer diff --git a/WebKitTools/Scripts/webkitpy/common/net/bugzilla/bug.py b/WebKitTools/Scripts/webkitpy/common/net/bugzilla/bug.py new file mode 100644 index 0000000..2cb4bc8 --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/common/net/bugzilla/bug.py @@ -0,0 +1,105 @@ +# Copyright (c) 2009 Google Inc. All rights reserved. +# Copyright (c) 2009 Apple Inc. All rights reserved. +# Copyright (c) 2010 Research In Motion Limited. 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. + +from .attachment import Attachment + + +class Bug(object): + # FIXME: This class is kinda a hack for now. It exists so we have one + # place to hold bug logic, even if much of the code deals with + # dictionaries still. + + def __init__(self, bug_dictionary, bugzilla): + self.bug_dictionary = bug_dictionary + self._bugzilla = bugzilla + + def id(self): + return self.bug_dictionary["id"] + + def title(self): + return self.bug_dictionary["title"] + + def assigned_to_email(self): + return self.bug_dictionary["assigned_to_email"] + + # FIXME: This information should be stored in some sort of webkit_config.py instead of here. + unassigned_emails = frozenset([ + "webkit-unassigned@lists.webkit.org", + "webkit-qt-unassigned@trolltech.com", + ]) + + def is_unassigned(self): + return self.assigned_to_email() in self.unassigned_emails + + def status(self): + return self.bug_dictionary["bug_status"] + + # Bugzilla has many status states we don't really use in WebKit: + # https://bugs.webkit.org/page.cgi?id=fields.html#status + _open_states = ["UNCONFIRMED", "NEW", "ASSIGNED", "REOPENED"] + _closed_states = ["RESOLVED", "VERIFIED", "CLOSED"] + + def is_open(self): + return self.status() in self._open_states + + def is_closed(self): + return not self.is_open() + + # Rarely do we actually want obsolete attachments + def attachments(self, include_obsolete=False): + attachments = self.bug_dictionary["attachments"] + if not include_obsolete: + attachments = filter(lambda attachment: + not attachment["is_obsolete"], attachments) + return [Attachment(attachment, self) for attachment in attachments] + + def patches(self, include_obsolete=False): + return [patch for patch in self.attachments(include_obsolete) + if patch.is_patch()] + + def unreviewed_patches(self): + return [patch for patch in self.patches() if patch.review() == "?"] + + def reviewed_patches(self, include_invalid=False): + patches = [patch for patch in self.patches() if patch.review() == "+"] + if include_invalid: + return patches + # Checking reviewer() ensures that it was both reviewed and has a valid + # reviewer. + return filter(lambda patch: patch.reviewer(), patches) + + def commit_queued_patches(self, include_invalid=False): + patches = [patch for patch in self.patches() + if patch.commit_queue() == "+"] + if include_invalid: + return patches + # Checking committer() ensures that it was both commit-queue+'d and has + # a valid committer. + return filter(lambda patch: patch.committer(), patches) diff --git a/WebKitTools/Scripts/webkitpy/common/net/bugzilla/bug_unittest.py b/WebKitTools/Scripts/webkitpy/common/net/bugzilla/bug_unittest.py new file mode 100644 index 0000000..d43d64f --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/common/net/bugzilla/bug_unittest.py @@ -0,0 +1,40 @@ +# Copyright (C) 2009 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 unittest + +from .bug import Bug + + +class BugTest(unittest.TestCase): + def test_is_unassigned(self): + for email in Bug.unassigned_emails: + bug = Bug({"assigned_to_email": email}, bugzilla=None) + self.assertTrue(bug.is_unassigned()) + bug = Bug({"assigned_to_email": "test@test.com"}, bugzilla=None) + self.assertFalse(bug.is_unassigned()) diff --git a/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py b/WebKitTools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py index a7dc1b7..9fa7fe5 100644 --- a/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py +++ b/WebKitTools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py @@ -36,15 +36,18 @@ import StringIO from datetime import datetime # used in timestamp() -from webkitpy.common.system.deprecated_logging import error, log +from .attachment import Attachment +from .bug import Bug + +from webkitpy.common.system.deprecated_logging import log from webkitpy.common.config import committers from webkitpy.common.net.credentials import Credentials -from webkitpy.common.system.ospath import relpath from webkitpy.common.system.user import User from webkitpy.thirdparty.autoinstalled.mechanize import Browser from webkitpy.thirdparty.BeautifulSoup import BeautifulSoup, SoupStrainer +# FIXME: parse_bug_id should not be a free function. def parse_bug_id(message): if not message: return None @@ -63,162 +66,6 @@ def timestamp(): return datetime.now().strftime("%Y%m%d%H%M%S") -class Attachment(object): - - rollout_preamble = "ROLLOUT of r" - - def __init__(self, attachment_dictionary, bug): - self._attachment_dictionary = attachment_dictionary - self._bug = bug - self._reviewer = None - self._committer = None - - def _bugzilla(self): - return self._bug._bugzilla - - def id(self): - return int(self._attachment_dictionary.get("id")) - - def attacher_is_committer(self): - return self._bugzilla.committers.committer_by_email( - patch.attacher_email()) - - def attacher_email(self): - return self._attachment_dictionary.get("attacher_email") - - def bug(self): - return self._bug - - def bug_id(self): - return int(self._attachment_dictionary.get("bug_id")) - - def is_patch(self): - return not not self._attachment_dictionary.get("is_patch") - - def is_obsolete(self): - return not not self._attachment_dictionary.get("is_obsolete") - - def is_rollout(self): - return self.name().startswith(self.rollout_preamble) - - def name(self): - return self._attachment_dictionary.get("name") - - def attach_date(self): - return self._attachment_dictionary.get("attach_date") - - def review(self): - return self._attachment_dictionary.get("review") - - def commit_queue(self): - return self._attachment_dictionary.get("commit-queue") - - def url(self): - # FIXME: This should just return - # self._bugzilla().attachment_url_for_id(self.id()). scm_unittest.py - # depends on the current behavior. - return self._attachment_dictionary.get("url") - - def contents(self): - # FIXME: We shouldn't be grabbing at _bugzilla. - return self._bug._bugzilla.fetch_attachment_contents(self.id()) - - def _validate_flag_value(self, flag): - email = self._attachment_dictionary.get("%s_email" % flag) - if not email: - return None - committer = getattr(self._bugzilla().committers, - "%s_by_email" % flag)(email) - if committer: - return committer - log("Warning, attachment %s on bug %s has invalid %s (%s)" % ( - self._attachment_dictionary['id'], - self._attachment_dictionary['bug_id'], flag, email)) - - def reviewer(self): - if not self._reviewer: - self._reviewer = self._validate_flag_value("reviewer") - return self._reviewer - - def committer(self): - if not self._committer: - self._committer = self._validate_flag_value("committer") - return self._committer - - -class Bug(object): - # FIXME: This class is kinda a hack for now. It exists so we have one - # place to hold bug logic, even if much of the code deals with - # dictionaries still. - - def __init__(self, bug_dictionary, bugzilla): - self.bug_dictionary = bug_dictionary - self._bugzilla = bugzilla - - def id(self): - return self.bug_dictionary["id"] - - def title(self): - return self.bug_dictionary["title"] - - def assigned_to_email(self): - return self.bug_dictionary["assigned_to_email"] - - # FIXME: This information should be stored in some sort of webkit_config.py instead of here. - unassigned_emails = frozenset([ - "webkit-unassigned@lists.webkit.org", - "webkit-qt-unassigned@trolltech.com", - ]) - def is_unassigned(self): - return self.assigned_to_email() in self.unassigned_emails - - def status(self): - return self.bug_dictionary["bug_status"] - - # Bugzilla has many status states we don't really use in WebKit: - # https://bugs.webkit.org/page.cgi?id=fields.html#status - _open_states = ["UNCONFIRMED", "NEW", "ASSIGNED", "REOPENED"] - _closed_states = ["RESOLVED", "VERIFIED", "CLOSED"] - - def is_open(self): - return self.status() in self._open_states - - def is_closed(self): - return not self.is_open() - - # Rarely do we actually want obsolete attachments - def attachments(self, include_obsolete=False): - attachments = self.bug_dictionary["attachments"] - if not include_obsolete: - attachments = filter(lambda attachment: - not attachment["is_obsolete"], attachments) - return [Attachment(attachment, self) for attachment in attachments] - - def patches(self, include_obsolete=False): - return [patch for patch in self.attachments(include_obsolete) - if patch.is_patch()] - - def unreviewed_patches(self): - return [patch for patch in self.patches() if patch.review() == "?"] - - def reviewed_patches(self, include_invalid=False): - patches = [patch for patch in self.patches() if patch.review() == "+"] - if include_invalid: - return patches - # Checking reviewer() ensures that it was both reviewed and has a valid - # reviewer. - return filter(lambda patch: patch.reviewer(), patches) - - def commit_queued_patches(self, include_invalid=False): - patches = [patch for patch in self.patches() - if patch.commit_queue() == "+"] - if include_invalid: - return patches - # Checking committer() ensures that it was both commit-queue+'d and has - # a valid committer. - return filter(lambda patch: patch.committer(), patches) - - # A container for all of the logic for making and parsing buzilla queries. class BugzillaQueries(object): @@ -298,92 +145,6 @@ class BugzillaQueries(object): return self._fetch_attachment_ids_request_query(review_queue_url) -class CommitterValidator(object): - - def __init__(self, bugzilla): - self._bugzilla = bugzilla - - # _view_source_url belongs in some sort of webkit_config.py module. - def _view_source_url(self, local_path): - return "http://trac.webkit.org/browser/trunk/%s" % local_path - - def _checkout_root(self): - # FIXME: This is a hack, we would have this from scm.checkout_root - # if we had any way to get to an scm object here. - components = __file__.split(os.sep) - tools_index = components.index("WebKitTools") - return os.sep.join(components[:tools_index]) - - def _committers_py_path(self): - # extension can sometimes be .pyc, we always want .py - (path, extension) = os.path.splitext(committers.__file__) - # FIXME: When we're allowed to use python 2.6 we can use the real - # os.path.relpath - path = relpath(path, self._checkout_root()) - return ".".join([path, "py"]) - - def _flag_permission_rejection_message(self, setter_email, flag_name): - # Should come from some webkit_config.py - contribution_guidlines = "http://webkit.org/coding/contributing.html" - # This could be queried from the status_server. - queue_administrator = "eseidel@chromium.org" - # This could be queried from the tool. - queue_name = "commit-queue" - committers_list = self._committers_py_path() - message = "%s does not have %s permissions according to %s." % ( - setter_email, - flag_name, - self._view_source_url(committers_list)) - message += "\n\n- If you do not have %s rights please read %s for instructions on how to use bugzilla flags." % ( - flag_name, contribution_guidlines) - message += "\n\n- If you have %s rights please correct the error in %s by adding yourself to the file (no review needed). " % ( - flag_name, committers_list) - message += "The %s restarts itself every 2 hours. After restart the %s will correctly respect your %s rights." % ( - queue_name, queue_name, flag_name) - return message - - def _validate_setter_email(self, patch, result_key, rejection_function): - committer = getattr(patch, result_key)() - # If the flag is set, and we don't recognize the setter, reject the - # flag! - setter_email = patch._attachment_dictionary.get("%s_email" % result_key) - if setter_email and not committer: - rejection_function(patch.id(), - self._flag_permission_rejection_message(setter_email, - result_key)) - return False - return True - - def _reject_patch_if_flags_are_invalid(self, patch): - return (self._validate_setter_email( - patch, "reviewer", self.reject_patch_from_review_queue) - and self._validate_setter_email( - patch, "committer", self.reject_patch_from_commit_queue)) - - def patches_after_rejecting_invalid_commiters_and_reviewers(self, patches): - return [patch for patch in patches if self._reject_patch_if_flags_are_invalid(patch)] - - def reject_patch_from_commit_queue(self, - attachment_id, - additional_comment_text=None): - comment_text = "Rejecting patch %s from commit-queue." % attachment_id - self._bugzilla.set_flag_on_attachment(attachment_id, - "commit-queue", - "-", - comment_text, - additional_comment_text) - - def reject_patch_from_review_queue(self, - attachment_id, - additional_comment_text=None): - comment_text = "Rejecting patch %s from review queue." % attachment_id - self._bugzilla.set_flag_on_attachment(attachment_id, - 'review', - '-', - comment_text, - additional_comment_text) - - class Bugzilla(object): def __init__(self, dryrun=False, committers=committers.CommitterList()): @@ -861,7 +622,6 @@ class Bugzilla(object): self.browser.open(self.bug_url_for_bug_id(bug_id)) self.browser.select_form(name="changeform") if comment_text: - log(comment_text) self.browser['comment'] = comment_text self.browser['bug_status'] = ['RESOLVED'] self.browser['resolution'] = ['FIXED'] diff --git a/WebKitTools/Scripts/webkitpy/common/net/bugzilla_unittest.py b/WebKitTools/Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py index df1fcf6..c476c81 100644 --- a/WebKitTools/Scripts/webkitpy/common/net/bugzilla_unittest.py +++ b/WebKitTools/Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py @@ -27,38 +27,16 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import unittest - import datetime -from webkitpy.common.config.committers import CommitterList, Reviewer, Committer -from webkitpy.common.net.bugzilla import Bugzilla, BugzillaQueries, parse_bug_id, CommitterValidator, Bug +from .bugzilla import Bugzilla, BugzillaQueries, parse_bug_id + from webkitpy.common.system.outputcapture import OutputCapture from webkitpy.tool.mocktool import MockBrowser from webkitpy.thirdparty.mock import Mock from webkitpy.thirdparty.BeautifulSoup import BeautifulSoup -class BugTest(unittest.TestCase): - def test_is_unassigned(self): - for email in Bug.unassigned_emails: - bug = Bug({"assigned_to_email" : email}, bugzilla=None) - self.assertTrue(bug.is_unassigned()) - bug = Bug({"assigned_to_email" : "test@test.com"}, bugzilla=None) - self.assertFalse(bug.is_unassigned()) - - -class CommitterValidatorTest(unittest.TestCase): - def test_flag_permission_rejection_message(self): - validator = CommitterValidator(bugzilla=None) - self.assertEqual(validator._committers_py_path(), "WebKitTools/Scripts/webkitpy/common/config/committers.py") - expected_messsage="""foo@foo.com does not have review permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py. - -- If you do not have review rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - -- If you have review rights please correct the error in WebKitTools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your review rights.""" - self.assertEqual(validator._flag_permission_rejection_message("foo@foo.com", "review"), expected_messsage) - - class BugzillaTest(unittest.TestCase): _example_attachment = ''' <attachment diff --git a/WebKitTools/Scripts/webkitpy/common/net/buildbot.py b/WebKitTools/Scripts/webkitpy/common/net/buildbot.py index a14bc7f..88cdd4e 100644 --- a/WebKitTools/Scripts/webkitpy/common/net/buildbot.py +++ b/WebKitTools/Scripts/webkitpy/common/net/buildbot.py @@ -220,6 +220,7 @@ class Build(object): results_html = "%s/results.html" % (self.results_url()) # FIXME: This should use NetworkTransaction's 404 handling instead. try: + # It seems this can return None if the url redirects and then returns 404. return urllib2.urlopen(results_html) except urllib2.HTTPError, error: if error.code != 404: diff --git a/WebKitTools/Scripts/webkitpy/common/net/buildbot_unittest.py b/WebKitTools/Scripts/webkitpy/common/net/buildbot_unittest.py index afc9a39..4fdf24c 100644 --- a/WebKitTools/Scripts/webkitpy/common/net/buildbot_unittest.py +++ b/WebKitTools/Scripts/webkitpy/common/net/buildbot_unittest.py @@ -111,6 +111,14 @@ class BuilderTest(unittest.TestCase): self.assertEqual(self.builder._revision_and_build_for_filename(filename), revision_and_build) +class BuildTest(unittest.TestCase): + def test_layout_test_results(self): + build = Build(None, None, None, None) + build._fetch_results_html = lambda: None + # Test that layout_test_results() returns None if the fetch fails. + self.assertEqual(build.layout_test_results(), None) + + class BuildBotTest(unittest.TestCase): _example_one_box_status = ''' diff --git a/WebKitTools/Scripts/webkitpy/common/net/layouttestresults.py b/WebKitTools/Scripts/webkitpy/common/net/layouttestresults.py index 2f7b3e6..a7b3b0a 100644 --- a/WebKitTools/Scripts/webkitpy/common/net/layouttestresults.py +++ b/WebKitTools/Scripts/webkitpy/common/net/layouttestresults.py @@ -58,6 +58,8 @@ class LayoutTestResults(object): @classmethod def _parse_results_html(cls, page): + if not page: + return None parsed_results = {} tables = BeautifulSoup(page).findAll("table") for table in tables: diff --git a/WebKitTools/Scripts/webkitpy/common/net/layouttestresults_unittest.py b/WebKitTools/Scripts/webkitpy/common/net/layouttestresults_unittest.py index 44e4dbc..8490eae 100644 --- a/WebKitTools/Scripts/webkitpy/common/net/layouttestresults_unittest.py +++ b/WebKitTools/Scripts/webkitpy/common/net/layouttestresults_unittest.py @@ -71,6 +71,7 @@ class LayoutTestResultsTest(unittest.TestCase): self.assertEqual(self._expected_layout_test_results, results) def test_results_from_string(self): + self.assertEqual(LayoutTestResults.results_from_string(None), None) self.assertEqual(LayoutTestResults.results_from_string(""), None) results = LayoutTestResults.results_from_string(self._example_results_html) self.assertEqual(len(results.failing_tests()), 0) diff --git a/WebKitTools/Scripts/webkitpy/common/newstringio.py b/WebKitTools/Scripts/webkitpy/common/newstringio.py new file mode 100644 index 0000000..f6d08ec --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/common/newstringio.py @@ -0,0 +1,40 @@ +#!/usr/bin/env python +# Copyright (C) 2010 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. + +"""'with'-compliant StringIO implementation.""" + +import StringIO + + +class StringIO(StringIO.StringIO): + def __enter__(self): + return self + + def __exit__(self, type, value, traceback): + pass diff --git a/WebKitTools/Scripts/webkitpy/common/newstringio_unittest.py b/WebKitTools/Scripts/webkitpy/common/newstringio_unittest.py new file mode 100644 index 0000000..5755c98 --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/common/newstringio_unittest.py @@ -0,0 +1,46 @@ +#!/usr/bin/env python +# Copyright (C) 2010 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. + +"""Unit tests for newstringio module.""" + +from __future__ import with_statement + +import unittest + +import newstringio + + +class NewStringIOTest(unittest.TestCase): + def test_with(self): + with newstringio.StringIO("foo") as f: + contents = f.read() + self.assertEqual(contents, "foo") + +if __name__ == '__main__': + unittest.main() diff --git a/WebKitTools/Scripts/webkitpy/common/system/executive.py b/WebKitTools/Scripts/webkitpy/common/system/executive.py index 37f4e53..85a683a 100644 --- a/WebKitTools/Scripts/webkitpy/common/system/executive.py +++ b/WebKitTools/Scripts/webkitpy/common/system/executive.py @@ -201,7 +201,7 @@ class Executive(object): return raise - def _win32_check_running_pid(self): + def _win32_check_running_pid(self, pid): class PROCESSENTRY32(ctypes.Structure): _fields_ = [("dwSize", ctypes.c_ulong), @@ -246,7 +246,7 @@ class Executive(object): except OSError: return False elif sys.platform == 'win32': - return self._win32_check_running_pid() + return self._win32_check_running_pid(pid) assert(False) diff --git a/WebKitTools/Scripts/webkitpy/common/system/executive_mock.py b/WebKitTools/Scripts/webkitpy/common/system/executive_mock.py new file mode 100644 index 0000000..7347ff9 --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/common/system/executive_mock.py @@ -0,0 +1,55 @@ +# Copyright (C) 2009 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. + +# FIXME: Implement the rest of the interface as needed for testing :). + +# FIXME: Unify with tool/mocktool.MockExecutive. + + +class MockExecutive2(object): + def __init__(self, output='', exit_code=0, exception=None): + self._output = output + self._exit_code = exit_code + self._exception = exception + + def cpu_count(self): + return 2 + + def kill_all(self, process_name): + pass + + def kill_process(self, pid): + pass + + def run_command(self, arg_list, return_exit_code=False, + decode_output=False): + if self._exception: + raise self._exception + if return_exit_code: + return self._exit_code + return self._output diff --git a/WebKitTools/Scripts/webkitpy/common/system/file_lock.py b/WebKitTools/Scripts/webkitpy/common/system/file_lock.py new file mode 100644 index 0000000..7296958 --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/common/system/file_lock.py @@ -0,0 +1,83 @@ +#!/usr/bin/env python +# Copyright (C) 2010 Gabor Rapcsanyi (rgabor@inf.u-szeged.hu), University of Szeged +# +# 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 UNIVERSITY OF SZEGED ``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 UNIVERSITY OF SZEGED 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. + +"""This class helps to lock files exclusively across processes.""" + +import logging +import os +import sys +import time + + +_log = logging.getLogger("webkitpy.common.system.file_lock") + + +class FileLock(object): + + def __init__(self, lock_file_path, max_wait_time_sec=20): + self._lock_file_path = lock_file_path + self._lock_file_descriptor = None + self._max_wait_time_sec = max_wait_time_sec + + def _create_lock(self): + if sys.platform in ('darwin', 'linux2', 'cygwin'): + import fcntl + fcntl.flock(self._lock_file_descriptor, fcntl.LOCK_EX | fcntl.LOCK_NB) + elif sys.platform == 'win32': + import msvcrt + msvcrt.locking(self._lock_file_descriptor, msvcrt.LK_NBLCK, 32) + + def _remove_lock(self): + if sys.platform in ('darwin', 'linux2', 'cygwin'): + import fcntl + fcntl.flock(self._lock_file_descriptor, fcntl.LOCK_UN) + elif sys.platform == 'win32': + import msvcrt + msvcrt.locking(self._lock_file_descriptor, msvcrt.LK_UNLCK, 32) + + def acquire_lock(self): + self._lock_file_descriptor = os.open(self._lock_file_path, os.O_TRUNC | os.O_CREAT) + start_time = time.time() + while True: + try: + self._create_lock() + return True + except IOError: + if time.time() - start_time > self._max_wait_time_sec: + _log.debug("File locking failed: %s" % str(sys.exc_info())) + os.close(self._lock_file_descriptor) + self._lock_file_descriptor = None + return False + + def release_lock(self): + try: + if self._lock_file_descriptor: + self._remove_lock() + os.close(self._lock_file_descriptor) + self._lock_file_descriptor = None + os.unlink(self._lock_file_path) + except (IOError, OSError): + _log.debug("Warning in release lock: %s" % str(sys.exc_info())) diff --git a/WebKitTools/Scripts/webkitpy/common/system/file_lock_unittest.py b/WebKitTools/Scripts/webkitpy/common/system/file_lock_unittest.py new file mode 100644 index 0000000..c5c1db3 --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/common/system/file_lock_unittest.py @@ -0,0 +1,61 @@ +#!/usr/bin/env python +# Copyright (C) 2010 Gabor Rapcsanyi (rgabor@inf.u-szeged.hu), University of Szeged +# +# 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 UNIVERSITY OF SZEGED ``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 UNIVERSITY OF SZEGED 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 +import tempfile +import unittest + +from webkitpy.common.system.file_lock import FileLock + + +class FileLockTest(unittest.TestCase): + + def setUp(self): + self._lock_name = "TestWebKit" + str(os.getpid()) + ".lock" + self._lock_path = os.path.join(tempfile.gettempdir(), self._lock_name) + self._file_lock1 = FileLock(self._lock_path, 1) + self._file_lock2 = FileLock(self._lock_path, 1) + + def tearDown(self): + self._file_lock1.release_lock() + self._file_lock2.release_lock() + + def test_lock_lifecycle(self): + # Create the lock. + self._file_lock1.acquire_lock() + self.assertTrue(os.path.exists(self._lock_path)) + + # Try to lock again. + self.assertFalse(self._file_lock2.acquire_lock()) + + # Release the lock. + self._file_lock1.release_lock() + self.assertFalse(os.path.exists(self._lock_path)) + + def test_stuck_lock(self): + open(self._lock_path, 'w').close() + self._file_lock1.acquire_lock() + self._file_lock1.release_lock() diff --git a/WebKitTools/Scripts/webkitpy/common/system/filesystem_mock.py b/WebKitTools/Scripts/webkitpy/common/system/filesystem_mock.py new file mode 100644 index 0000000..d2cde4f --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/common/system/filesystem_mock.py @@ -0,0 +1,75 @@ +# Copyright (C) 2009 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 errno +import os +import path + + +class MockFileSystem(object): + def __init__(self, files={}): + """Initializes a "mock" filesystem that can be used to completely + stub out a filesystem. + + Args: + files: a dict of filenames -> file contents. A file contents + value of None is used to indicate that the file should + not exist (even if standalone is False). + standalone: If True, only the files listed in _files_ exist. + If False, the object will pass through read calls to the + underlying filesystem. Writes are never passed through. + + """ + self.files = files + + def exists(self, path): + if path in self.files: + return self.files[path] is not None + return False + + def join(self, *comps): + return '/'.join(comps) + + def maybe_make_directory(self, *path): + # FIXME: Implement such that subsequent calls to isdir() work? + pass + + def read_text_file(self, path): + return self.read_binary_file(path) + + def read_binary_file(self, path): + if path in self.files: + if self.files[path] is None: + raise IOError(errno.ENOENT, path, os.strerror(errno.ENOENT)) + return self.files[path] + + def write_text_file(self, path, contents): + return self.write_binary_file(path, contents) + + def write_binary_file(self, path, contents): + self.files[path] = contents diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py index 9f2de7e..88f493d 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py @@ -51,6 +51,7 @@ import time import traceback import test_failures +import test_output import test_results _log = logging.getLogger("webkitpy.layout_tests.layout_package." @@ -74,9 +75,14 @@ def log_stack(stack): _log.error(' %s' % line.strip()) -def _process_output(port, options, test_info, test_types, test_args, - crash, timeout, test_run_time, actual_checksum, - output, error): +def _expected_test_output(port, filename): + """Returns an expected TestOutput object.""" + return test_output.TestOutput(port.expected_text(filename), + port.expected_image(filename), + port.expected_checksum(filename)) + +def _process_output(port, options, test_input, test_types, test_args, + test_output): """Receives the output from a DumpRenderTree process, subjects it to a number of tests, and returns a list of failure types the test produced. @@ -84,57 +90,55 @@ def _process_output(port, options, test_info, test_types, test_args, port: port-specific hooks options: command line options argument from optparse proc: an active DumpRenderTree process - test_info: Object containing the test filename, uri and timeout + test_input: Object containing the test filename and timeout test_types: list of test types to subject the output to test_args: arguments to be passed to each test + test_output: a TestOutput object containing the output of the test Returns: a TestResult object """ failures = [] - # Some test args, such as the image hash, may be added or changed on a - # test-by-test basis. - local_test_args = copy.copy(test_args) - - local_test_args.hash = actual_checksum - - if crash: + if test_output.crash: failures.append(test_failures.FailureCrash()) - if timeout: + if test_output.timeout: failures.append(test_failures.FailureTimeout()) - if crash: - _log.debug("Stacktrace for %s:\n%s" % (test_info.filename, error)) + if test_output.crash: + _log.debug("Stacktrace for %s:\n%s" % (test_input.filename, + test_output.error)) # Strip off "file://" since RelativeTestFilename expects # filesystem paths. filename = os.path.join(options.results_directory, port.relative_test_filename( - test_info.filename)) + test_input.filename)) filename = os.path.splitext(filename)[0] + "-stack.txt" port.maybe_make_directory(os.path.split(filename)[0]) with codecs.open(filename, "wb", "utf-8") as file: - file.write(error) - elif error: - _log.debug("Previous test output stderr lines:\n%s" % error) + file.write(test_output.error) + elif test_output.error: + _log.debug("Previous test output stderr lines:\n%s" % test_output.error) + + expected_test_output = _expected_test_output(port, test_input.filename) # Check the output and save the results. start_time = time.time() time_for_diffs = {} for test_type in test_types: start_diff_time = time.time() - new_failures = test_type.compare_output(port, test_info.filename, - output, local_test_args, - options.configuration) + new_failures = test_type.compare_output(port, test_input.filename, + test_args, test_output, + expected_test_output) # Don't add any more failures if we already have a crash, so we don't # double-report those tests. We do double-report for timeouts since # we still want to see the text and image output. - if not crash: + if not test_output.crash: failures.extend(new_failures) time_for_diffs[test_type.__class__.__name__] = ( time.time() - start_diff_time) total_time_for_all_diffs = time.time() - start_diff_time - return test_results.TestResult(test_info.filename, failures, test_run_time, + return test_results.TestResult(test_input.filename, failures, test_output.test_time, total_time_for_all_diffs, time_for_diffs) @@ -153,22 +157,36 @@ def _milliseconds_to_seconds(msecs): return float(msecs) / 1000.0 -def _image_hash(test_info, test_args, options): - """Returns the image hash of the test if it's needed, otherwise None.""" - if (test_args.new_baseline or test_args.reset_results or not options.pixel_tests): - return None - return test_info.image_hash() +def _should_fetch_expected_checksum(options): + return options.pixel_tests and not (options.new_baseline or options.reset_results) + + +def _run_single_test(port, options, test_input, test_types, test_args, driver): + # FIXME: Pull this into TestShellThread._run(). + + # The image hash is used to avoid doing an image dump if the + # checksums match, so it should be set to a blank value if we + # are generating a new baseline. (Otherwise, an image from a + # previous run will be copied into the baseline.""" + if _should_fetch_expected_checksum(options): + image_hash_to_driver = port.expected_checksum(test_input.filename) + else: + image_hash_to_driver = None + uri = port.filename_to_uri(test_input.filename) + test_output = driver.run_test(uri, test_input.timeout, image_hash_to_driver) + return _process_output(port, options, test_input, test_types, test_args, + test_output) class SingleTestThread(threading.Thread): """Thread wrapper for running a single test file.""" - def __init__(self, port, options, test_info, test_types, test_args): + def __init__(self, port, options, test_input, test_types, test_args): """ Args: port: object implementing port-specific hooks options: command line argument object from optparse - test_info: Object containing the test filename, uri and timeout + test_input: Object containing the test filename and timeout test_types: A list of TestType objects to run the test output against. test_args: A TestArguments object to pass to each TestType. @@ -177,7 +195,7 @@ class SingleTestThread(threading.Thread): threading.Thread.__init__(self) self._port = port self._options = options - self._test_info = test_info + self._test_input = test_input self._test_types = test_types self._test_args = test_args self._driver = None @@ -188,20 +206,12 @@ class SingleTestThread(threading.Thread): def _covered_run(self): # FIXME: this is a separate routine to work around a bug # in coverage: see http://bitbucket.org/ned/coveragepy/issue/85. - test_info = self._test_info self._driver = self._port.create_driver(self._test_args.png_path, self._options) self._driver.start() - image_hash = _image_hash(test_info, self._test_args, self._options) - start = time.time() - crash, timeout, actual_checksum, output, error = \ - self._driver.run_test(test_info.uri.strip(), test_info.timeout, - image_hash) - end = time.time() - self._test_result = _process_output(self._port, self._options, - test_info, self._test_types, self._test_args, - crash, timeout, end - start, - actual_checksum, output, error) + self._test_result = _run_single_test(self._port, self._options, + self._test_input, self._test_types, + self._test_args, self._driver) self._driver.stop() def get_test_result(self): @@ -258,7 +268,6 @@ class TestShellThread(WatchableThread): test_types: A list of TestType objects to run the test output against. test_args: A TestArguments object to pass to each TestType. - """ WatchableThread.__init__(self) self._port = port @@ -402,17 +411,17 @@ class TestShellThread(WatchableThread): self._num_tests_in_current_group = len(self._filename_list) self._current_group_start_time = time.time() - test_info = self._filename_list.pop() + test_input = self._filename_list.pop() # We have a url, run tests. batch_count += 1 self._num_tests += 1 if self._options.run_singly: - result = self._run_test_singly(test_info) + result = self._run_test_singly(test_input) else: - result = self._run_test(test_info) + result = self._run_test(test_input) - filename = test_info.filename + filename = test_input.filename tests_run_file.write(filename + "\n") if result.failures: # Check and kill DumpRenderTree if we need to. @@ -440,7 +449,7 @@ class TestShellThread(WatchableThread): if test_runner: test_runner.update_summary(result_summary) - def _run_test_singly(self, test_info): + def _run_test_singly(self, test_input): """Run a test in a separate thread, enforcing a hard time limit. Since we can only detect the termination of a thread, not any internal @@ -448,7 +457,7 @@ class TestShellThread(WatchableThread): files singly. Args: - test_info: Object containing the test filename, uri and timeout + test_input: Object containing the test filename and timeout Returns: A TestResult @@ -456,14 +465,14 @@ class TestShellThread(WatchableThread): """ worker = SingleTestThread(self._port, self._options, - test_info, + test_input, self._test_types, self._test_args) worker.start() thread_timeout = _milliseconds_to_seconds( - _pad_timeout(int(test_info.timeout))) + _pad_timeout(int(test_input.timeout))) thread._next_timeout = time.time() + thread_timeout worker.join(thread_timeout) if worker.isAlive(): @@ -485,43 +494,29 @@ class TestShellThread(WatchableThread): # This gets raised if the worker thread has already exited. failures = [] _log.error('Cannot get results of test: %s' % - test_info.filename) - result = test_results.TestResult(test_info.filename, failures=[], + test_input.filename) + result = test_results.TestResult(test_input.filename, failures=[], test_run_time=0, total_time_for_all_diffs=0, time_for_diffs=0) return result - def _run_test(self, test_info): + def _run_test(self, test_input): """Run a single test file using a shared DumpRenderTree process. Args: - test_info: Object containing the test filename, uri and timeout + test_input: Object containing the test filename, uri and timeout Returns: a TestResult object. """ self._ensure_dump_render_tree_is_running() - # The pixel_hash is used to avoid doing an image dump if the - # checksums match, so it should be set to a blank value if we - # are generating a new baseline. (Otherwise, an image from a - # previous run will be copied into the baseline.) - image_hash = _image_hash(test_info, self._test_args, self._options) - start = time.time() - thread_timeout = _milliseconds_to_seconds( - _pad_timeout(int(test_info.timeout))) - self._next_timeout = start + thread_timeout - - crash, timeout, actual_checksum, output, error = \ - self._driver.run_test(test_info.uri, test_info.timeout, image_hash) - end = time.time() - - result = _process_output(self._port, self._options, - test_info, self._test_types, - self._test_args, crash, - timeout, end - start, actual_checksum, - output, error) - self._test_results.append(result) - return result + _pad_timeout(int(test_input.timeout))) + self._next_timeout = time.time() + thread_timeout + test_result = _run_single_test(self._port, self._options, test_input, + self._test_types, self._test_args, + self._driver) + self._test_results.append(test_result) + return test_result def _ensure_dump_render_tree_is_running(self): """Start the shared DumpRenderTree, if it's not running. diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_layout_results_generator.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_layout_results_generator.py index 1cf88ef..101d30b 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_layout_results_generator.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_layout_results_generator.py @@ -56,7 +56,8 @@ class JSONLayoutResultsGenerator(json_results_generator.JSONResultsGeneratorBase def __init__(self, port, builder_name, build_name, build_number, results_file_base_path, builder_base_url, test_timings, expectations, result_summary, all_tests, - generate_incremental_results=False, test_results_server=None): + generate_incremental_results=False, test_results_server=None, + test_type="", master_name=""): """Modifies the results.json file. Grabs it off the archive directory if it is not found locally. @@ -67,7 +68,8 @@ class JSONLayoutResultsGenerator(json_results_generator.JSONResultsGeneratorBase super(JSONLayoutResultsGenerator, self).__init__( builder_name, build_name, build_number, results_file_base_path, builder_base_url, {}, port.test_repository_paths(), - generate_incremental_results, test_results_server) + generate_incremental_results, test_results_server, + test_type, master_name) self._port = port self._expectations = expectations @@ -117,7 +119,7 @@ class JSONLayoutResultsGenerator(json_results_generator.JSONResultsGeneratorBase return set(self._failures.keys()) # override - def _get_result_type_char(self, test_name): + def _get_modifier_char(self, test_name): if test_name not in self._all_tests: return self.NO_DATA_RESULT diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py index 765b4d8..3267718 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py @@ -46,17 +46,35 @@ import webkitpy.thirdparty.simplejson as simplejson _log = logging.getLogger("webkitpy.layout_tests.layout_package.json_results_generator") - class TestResult(object): """A simple class that represents a single test result.""" - def __init__(self, name, failed=False, skipped=False, elapsed_time=0): + + # Test modifier constants. + (NONE, FAILS, FLAKY, DISABLED) = range(4) + + def __init__(self, name, failed=False, elapsed_time=0): self.name = name self.failed = failed - self.skipped = skipped self.time = elapsed_time + test_name = name + try: + test_name = name.split('.')[1] + except IndexError: + _log.warn("Invalid test name: %s.", name) + pass + + if test_name.startswith('FAILS_'): + self.modifier = self.FAILS + elif test_name.startswith('FLAKY_'): + self.modifier = self.FLAKY + elif test_name.startswith('DISABLED_'): + self.modifier = self.DISABLED + else: + self.modifier = self.NONE + def fixable(self): - return self.failed or self.skipped + return self.failed or self.modifier == self.DISABLED class JSONResultsGeneratorBase(object): @@ -67,10 +85,20 @@ class JSONResultsGeneratorBase(object): MIN_TIME = 1 JSON_PREFIX = "ADD_RESULTS(" JSON_SUFFIX = ");" + + # Note that in non-chromium tests those chars are used to indicate + # test modifiers (FAILS, FLAKY, etc) but not actual test results. PASS_RESULT = "P" SKIP_RESULT = "X" FAIL_RESULT = "F" + FLAKY_RESULT = "L" NO_DATA_RESULT = "N" + + MODIFIER_TO_CHAR = {TestResult.NONE: PASS_RESULT, + TestResult.DISABLED: SKIP_RESULT, + TestResult.FAILS: FAIL_RESULT, + TestResult.FLAKY: FLAKY_RESULT} + VERSION = 3 VERSION_KEY = "version" RESULTS = "results" @@ -94,7 +122,8 @@ class JSONResultsGeneratorBase(object): test_results_map, svn_repositories=None, generate_incremental_results=False, test_results_server=None, - test_type=""): + test_type="", + master_name=""): """Modifies the results.json file. Grabs it off the archive directory if it is not found locally. @@ -113,11 +142,14 @@ class JSONResultsGeneratorBase(object): generate_incremental_results: If true, generate incremental json file from current run results. test_results_server: server that hosts test results json. + test_type: test type string (e.g. 'layout-tests'). + master_name: the name of the buildbot master. """ self._builder_name = builder_name self._build_name = build_name self._build_number = build_number self._builder_base_url = builder_base_url + self._results_directory = results_file_base_path self._results_file_path = os.path.join(results_file_base_path, self.RESULTS_FILENAME) self._incremental_results_file_path = os.path.join( @@ -133,6 +165,7 @@ class JSONResultsGeneratorBase(object): self._test_results_server = test_results_server self._test_type = test_type + self._master_name = master_name self._json = None self._archived_results = None @@ -205,6 +238,36 @@ class JSONResultsGeneratorBase(object): def set_archived_results(self, archived_results): self._archived_results = archived_results + def upload_json_files(self, json_files): + """Uploads the given json_files to the test_results_server (if the + test_results_server is given).""" + if not self._test_results_server: + return + + if not self._master_name: + _log.error("--test-results-server was set, but --master-name was not. Not uploading JSON files.") + return + + _log.info("Uploading JSON files for builder: %s", self._builder_name) + attrs = [("builder", self._builder_name), + ("testtype", self._test_type), + ("master", self._master_name)] + + files = [(file, os.path.join(self._results_directory, file)) + for file in json_files] + + uploader = test_results_uploader.TestResultsUploader( + self._test_results_server) + try: + # Set uploading timeout in case appengine server is having problem. + # 120 seconds are more than enough to upload test results. + uploader.upload(attrs, files, 120) + except Exception, err: + _log.error("Upload failed: %s" % err) + return + + _log.info("JSON files uploaded.") + def _generate_json_file(self, json, file_path): # Specify separators in order to get compact encoding. json_data = simplejson.dumps(json, separators=(',', ':')) @@ -226,19 +289,17 @@ class JSONResultsGeneratorBase(object): """Returns a set of failed test names.""" return set([r.name for r in self._test_results if r.failed]) - def _get_result_type_char(self, test_name): + def _get_modifier_char(self, test_name): """Returns a single char (e.g. SKIP_RESULT, FAIL_RESULT, - PASS_RESULT, NO_DATA_RESULT, etc) that indicates the test result + PASS_RESULT, NO_DATA_RESULT, etc) that indicates the test modifier for the given test_name. """ if test_name not in self._test_results_map: return JSONResultsGenerator.NO_DATA_RESULT test_result = self._test_results_map[test_name] - if test_result.skipped: - return JSONResultsGenerator.SKIP_RESULT - if test_result.failed: - return JSONResultsGenerator.FAIL_RESULT + if test_result.modifier in self.MODIFIER_TO_CHAR.keys(): + return self.MODIFIER_TO_CHAR[test_result.modifier] return JSONResultsGenerator.PASS_RESULT @@ -344,10 +405,10 @@ class JSONResultsGeneratorBase(object): self._insert_item_into_raw_list(results_for_builder, fixable_count, self.FIXABLE_COUNT) - # Create a pass/skip/failure summary dictionary. + # Create a test modifiers (FAILS, FLAKY etc) summary dictionary. entry = {} for test_name in self._test_results_map.iterkeys(): - result_char = self._get_result_type_char(test_name) + result_char = self._get_modifier_char(test_name) entry[result_char] = entry.get(result_char, 0) + 1 # Insert the pass/skip/failure summary dictionary. @@ -423,7 +484,7 @@ class JSONResultsGeneratorBase(object): tests: Dictionary containing test result entries. """ - result = self._get_result_type_char(test_name) + result = self._get_modifier_char(test_name) time = self._get_test_timing(test_name) if test_name not in tests: @@ -523,33 +584,10 @@ class JSONResultsGenerator(JSONResultsGeneratorBase): # The flag is for backward compatibility. output_json_in_init = True - def _upload_json_files(self): - if not self._test_results_server or not self._test_type: - return - - _log.info("Uploading JSON files for %s to the server: %s", - self._builder_name, self._test_results_server) - attrs = [("builder", self._builder_name), ("testtype", self._test_type)] - json_files = [self.INCREMENTAL_RESULTS_FILENAME] - - files = [(file, os.path.join(self._results_directory, file)) - for file in json_files] - uploader = test_results_uploader.TestResultsUploader( - self._test_results_server) - try: - # Set uploading timeout in case appengine server is having problem. - # 120 seconds are more than enough to upload test results. - uploader.upload(attrs, files, 120) - except Exception, err: - _log.error("Upload failed: %s" % err) - return - - _log.info("JSON files uploaded.") - def __init__(self, port, builder_name, build_name, build_number, results_file_base_path, builder_base_url, test_timings, failures, passed_tests, skipped_tests, all_tests, - test_results_server=None, test_type=None): + test_results_server=None, test_type=None, master_name=None): """Generates a JSON results file. Args @@ -567,6 +605,7 @@ class JSONResultsGenerator(JSONResultsGeneratorBase): include skipped tests. test_results_server: server that hosts test results json. test_type: the test type. + master_name: the name of the buildbot master. """ self._test_type = test_type @@ -582,11 +621,9 @@ class JSONResultsGenerator(JSONResultsGeneratorBase): test_result.failed = True for test in skipped_tests: test_results_map[test] = test_result = get(test, TestResult(test)) - test_result.skipped = True for test in passed_tests: test_results_map[test] = test_result = get(test, TestResult(test)) test_result.failed = False - test_result.skipped = False for test in all_tests: if test not in test_results_map: test_results_map[test] = TestResult(test) @@ -599,8 +636,9 @@ class JSONResultsGenerator(JSONResultsGeneratorBase): svn_repositories=port.test_repository_paths(), generate_incremental_results=True, test_results_server=test_results_server, - test_type=test_type) + test_type=test_type, + master_name=master_name) if self.__class__.output_json_in_init: self.generate_json_output() - self._upload_json_files() + self.upload_json_files([self.INCREMENTAL_RESULTS_FILENAME]) diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator_unittest.py index 785cc1c..606a613 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator_unittest.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator_unittest.py @@ -47,43 +47,68 @@ class JSONGeneratorTest(unittest.TestCase): self.build_number = 'DUMMY_BUILDER_NUMBER' self._json = None self._num_runs = 0 - self._tests_list = set([]) + self._tests_set = set([]) self._test_timings = {} - self._failed_tests = {} - self._passed_tests = set([]) - self._skipped_tests = set([]) - - def _test_json_generation(self, passed_tests, failed_tests, skipped_tests): - # Make sure we have sets (rather than lists). - passed_tests = set(passed_tests) - skipped_tests = set(skipped_tests) - tests_list = passed_tests | set(failed_tests.keys()) + self._failed_tests = set([]) + + self._PASS_tests = set([]) + self._DISABLED_tests = set([]) + self._FLAKY_tests = set([]) + self._FAILS_tests = set([]) + + def _get_test_modifier(self, test_name): + if test_name.startswith('DISABLED_'): + return json_results_generator.JSONResultsGenerator.SKIP_RESULT + elif test_name.startswith('FLAKY_'): + return json_results_generator.JSONResultsGenerator.FLAKY_RESULT + elif test_name.startswith('FAILS_'): + return json_results_generator.JSONResultsGenerator.FAIL_RESULT + return json_results_generator.JSONResultsGenerator.PASS_RESULT + + def _test_json_generation(self, passed_tests_list, failed_tests_list): + tests_set = set(passed_tests_list) | set(failed_tests_list) + + DISABLED_tests = set([t for t in tests_set + if t.startswith('DISABLED_')]) + FLAKY_tests = set([t for t in tests_set + if t.startswith('FLAKY_')]) + FAILS_tests = set([t for t in tests_set + if t.startswith('FAILS_')]) + PASS_tests = tests_set ^ (DISABLED_tests | FLAKY_tests | FAILS_tests) + + passed_tests = set(passed_tests_list) ^ DISABLED_tests + failed_tests = set(failed_tests_list) + test_timings = {} i = 0 - for test in tests_list: + for test in tests_set: test_timings[test] = float(self._num_runs * 100 + i) i += 1 - port_obj = port.get(None) + # For backward compatibility. + reason = test_expectations.TEXT + failed_tests_dict = dict([(name, reason) for name in failed_tests]) + port_obj = port.get(None) generator = json_results_generator.JSONResultsGenerator(port_obj, self.builder_name, self.build_name, self.build_number, '', None, # don't fetch past json results archive test_timings, - failed_tests, + failed_tests_dict, passed_tests, - skipped_tests, - tests_list) + (), + tests_set) # Test incremental json results incremental_json = generator.get_json(incremental=True) self._verify_json_results( - tests_list, + tests_set, test_timings, - passed_tests, failed_tests, - skipped_tests, + PASS_tests, + DISABLED_tests, + FLAKY_tests, incremental_json, 1) @@ -92,23 +117,25 @@ class JSONGeneratorTest(unittest.TestCase): json = generator.get_json(incremental=False) self._json = json self._num_runs += 1 - self._tests_list |= tests_list + self._tests_set |= tests_set self._test_timings.update(test_timings) self._failed_tests.update(failed_tests) - self._passed_tests |= passed_tests - self._skipped_tests |= skipped_tests + self._PASS_tests |= PASS_tests + self._DISABLED_tests |= DISABLED_tests + self._FLAKY_tests |= FLAKY_tests self._verify_json_results( - self._tests_list, + self._tests_set, self._test_timings, - self._passed_tests, self._failed_tests, - self._skipped_tests, + self._PASS_tests, + self._DISABLED_tests, + self._FLAKY_tests, self._json, self._num_runs) - def _verify_json_results(self, tests_list, test_timings, - passed_tests, failed_tests, - skipped_tests, json, num_runs): + def _verify_json_results(self, tests_set, test_timings, failed_tests, + PASS_tests, DISABLED_tests, FLAKY_tests, + json, num_runs): # Aliasing to a short name for better access to its constants. JRG = json_results_generator.JSONResultsGenerator @@ -118,10 +145,10 @@ class JSONGeneratorTest(unittest.TestCase): buildinfo = json[self.builder_name] self.assertTrue(JRG.FIXABLE in buildinfo) self.assertTrue(JRG.TESTS in buildinfo) - self.assertTrue(len(buildinfo[JRG.BUILD_NUMBERS]) == num_runs) - self.assertTrue(buildinfo[JRG.BUILD_NUMBERS][0] == self.build_number) + self.assertEqual(len(buildinfo[JRG.BUILD_NUMBERS]), num_runs) + self.assertEqual(buildinfo[JRG.BUILD_NUMBERS][0], self.build_number) - if tests_list or skipped_tests: + if tests_set or DISABLED_tests: fixable = {} for fixable_items in buildinfo[JRG.FIXABLE]: for (type, count) in fixable_items.iteritems(): @@ -130,52 +157,58 @@ class JSONGeneratorTest(unittest.TestCase): else: fixable[type] = count - if passed_tests: - self.assertTrue(fixable[JRG.PASS_RESULT] == len(passed_tests)) + if PASS_tests: + self.assertEqual(fixable[JRG.PASS_RESULT], len(PASS_tests)) else: self.assertTrue(JRG.PASS_RESULT not in fixable or fixable[JRG.PASS_RESULT] == 0) - if skipped_tests: - self.assertTrue(fixable[JRG.SKIP_RESULT] == len(skipped_tests)) + if DISABLED_tests: + self.assertEqual(fixable[JRG.SKIP_RESULT], len(DISABLED_tests)) else: self.assertTrue(JRG.SKIP_RESULT not in fixable or fixable[JRG.SKIP_RESULT] == 0) + if FLAKY_tests: + self.assertEqual(fixable[JRG.FLAKY_RESULT], len(FLAKY_tests)) + else: + self.assertTrue(JRG.FLAKY_RESULT not in fixable or + fixable[JRG.FLAKY_RESULT] == 0) if failed_tests: tests = buildinfo[JRG.TESTS] - for test_name, failure in failed_tests.iteritems(): + for test_name in failed_tests: self.assertTrue(test_name in tests) test = tests[test_name] failed = 0 + modifier = self._get_test_modifier(test_name) for result in test[JRG.RESULTS]: - if result[1] == JRG.FAIL_RESULT: + if result[1] == modifier: failed = result[0] - self.assertTrue(failed == 1) + self.assertEqual(1, failed) timing_count = 0 for timings in test[JRG.TIMES]: if timings[1] == test_timings[test_name]: timing_count = timings[0] - self.assertTrue(timing_count == 1) + self.assertEqual(1, timing_count) - fixable_count = len(skipped_tests) + len(failed_tests.keys()) - if skipped_tests or failed_tests: - self.assertTrue(sum(buildinfo[JRG.FIXABLE_COUNT]) == fixable_count) + fixable_count = len(DISABLED_tests | failed_tests) + if DISABLED_tests or failed_tests: + self.assertEqual(sum(buildinfo[JRG.FIXABLE_COUNT]), fixable_count) def test_json_generation(self): - reason = test_expectations.TEXT - - self._test_json_generation([], {}, []) - self._test_json_generation(['A1', 'B1'], {}, []) - self._test_json_generation([], {'A2': reason, 'B2': reason}, []) - self._test_json_generation([], {}, ['A3', 'B3']) - self._test_json_generation(['A4'], {'B4': reason, 'C4': reason}, []) + self._test_json_generation([], []) + self._test_json_generation(['A1', 'B1'], []) + self._test_json_generation([], ['FAILS_A2', 'FAILS_B2']) + self._test_json_generation(['DISABLED_A3', 'DISABLED_B3'], []) + self._test_json_generation(['A4'], ['B4', 'FAILS_C4']) + self._test_json_generation(['DISABLED_C5', 'DISABLED_D5'], ['A5', 'B5']) self._test_json_generation( - [], {'A5': reason, 'B5': reason}, ['C5', 'D5']) + ['A6', 'B6', 'FAILS_C6', 'DISABLED_E6', 'DISABLED_F6'], + ['FAILS_D6']) self._test_json_generation( - ['A6', 'B6', 'C6'], {'D6': reason}, ['E6', 'F6']) - + ['A7', 'FLAKY_B7', 'DISABLED_C7'], + ['FAILS_D7', 'FLAKY_D8']) if __name__ == '__main__': unittest.main() diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/printing.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/printing.py index 00ff211..fb9fe6d 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/printing.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/printing.py @@ -182,8 +182,8 @@ def _configure_logging(stream, verbose): log_datefmt = '%y%m%d %H:%M:%S' log_level = logging.INFO if verbose: - log_fmt = ('%(asctime)s %(filename)s:%(lineno)-4d %(levelname)s ' - '%(message)s') + log_fmt = ('%(asctime)s %(process)d %(filename)s:%(lineno)-4d %(levelname)s' + '%(message)s') log_level = logging.DEBUG root = logging.getLogger() diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_output.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_output.py new file mode 100644 index 0000000..e809be6 --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_output.py @@ -0,0 +1,56 @@ +# Copyright (C) 2010 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. + + +class TestOutput(object): + """Groups information about a test output for easy passing of data. + + This is used not only for a actual test output, but also for grouping + expected test output. + """ + + def __init__(self, text, image, image_hash, + crash=None, test_time=None, timeout=None, error=None): + """Initializes a TestOutput object. + + Args: + text: a text output + image: an image output + image_hash: a string containing the checksum of the image + crash: a boolean indicating whether the driver crashed on the test + test_time: a time which the test has taken + timeout: a boolean indicating whehter the test timed out + error: any unexpected or additional (or error) text output + """ + self.text = text + self.image = image + self.image_hash = image_hash + self.crash = crash + self.test_time = test_time + self.timeout = timeout + self.error = error diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py index a98b858..632806f 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py @@ -30,10 +30,7 @@ """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 codecs import difflib import errno import os @@ -42,15 +39,17 @@ import sys import time import apache_http_server +import config as port_config import http_lock import http_server import test_files import websocket_server -from webkitpy.common.memoized import memoized +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.path import abspath_to_uri from webkitpy.common.system.user import User @@ -75,27 +74,26 @@ class DummyOptions(object): # FIXME: This class should merge with webkitpy.webkit_port at some point. class Port(object): - """Abstract class for Port-specific hooks for the layout_test package. - """ - - @staticmethod - def flag_from_configuration(configuration): - flags_by_configuration = { - "Debug": "--debug", - "Release": "--release", - } - return flags_by_configuration[configuration] - - def __init__(self, **kwargs): - self._name = kwargs.get('port_name', None) - self._options = kwargs.get('options') + """Abstract class for Port-specific hooks for the layout_test package.""" + + def __init__(self, port_name=None, options=None, + executive=None, + user=None, + filesystem=None, + config=None, + **kwargs): + self._name = port_name + self._options = options if self._options is None: # FIXME: Ideally we'd have a package-wide way to get a # well-formed options object that had all of the necessary # options defined on it. self._options = DummyOptions() - self._executive = kwargs.get('executive', Executive()) - self._user = kwargs.get('user', User()) + self._executive = executive or Executive() + self._user = user or User() + self._filesystem = filesystem or system.filesystem.FileSystem() + self._config = config or port_config.Config(self._executive, + self._filesystem) self._helper = None self._http_server = None self._webkit_base_dir = None @@ -118,7 +116,7 @@ class Port(object): self._wdiff_available = True self._pretty_patch_path = self.path_from_webkit_base("BugsSite", - "PrettyPatch", "prettify.rb") + "PrettyPatch", "prettify.rb") self._pretty_patch_available = True self.set_option_default('configuration', None) if self._options.configuration is None: @@ -265,7 +263,8 @@ class Port(object): baselines = [] for platform_dir in baseline_search_path: - if os.path.exists(os.path.join(platform_dir, baseline_filename)): + if self.path_exists(self._filesystem.join(platform_dir, + baseline_filename)): baselines.append((platform_dir, baseline_filename)) if not all_baselines and baselines: @@ -274,7 +273,8 @@ class Port(object): # If it wasn't found in a platform directory, return the expected # result in the test directory, even if no such file actually exists. platform_dir = self.layout_tests_dir() - if os.path.exists(os.path.join(platform_dir, baseline_filename)): + if self.path_exists(self._filesystem.join(platform_dir, + baseline_filename)): baselines.append((platform_dir, baseline_filename)) if baselines: @@ -304,35 +304,32 @@ class Port(object): platform_dir, baseline_filename = self.expected_baselines( filename, suffix)[0] if platform_dir: - return os.path.join(platform_dir, baseline_filename) - return os.path.join(self.layout_tests_dir(), baseline_filename) - - def _expected_file_contents(self, test, extension, encoding): - path = self.expected_filename(test, extension) - if not os.path.exists(path): - return None - open_mode = 'r' - if encoding is None: - open_mode = 'r+b' - with codecs.open(path, open_mode, encoding) as file: - return file.read() + return self._filesystem.join(platform_dir, baseline_filename) + return self._filesystem.join(self.layout_tests_dir(), baseline_filename) 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.""" - return self._expected_file_contents(test, '.checksum', 'ascii') + path = self.expected_filename(test, '.checksum') + if not self.path_exists(path): + return None + return self._filesystem.read_text_file(path) def expected_image(self, test): """Returns the image we expect the test to produce.""" - return self._expected_file_contents(test, '.png', None) + path = self.expected_filename(test, '.png') + if not self.path_exists(path): + return None + return self._filesystem.read_binary_file(path) def expected_text(self, test): """Returns the text output we expect the test to produce.""" - # NOTE: -expected.txt files are ALWAYS utf-8. However, - # we do not decode the output from DRT, so we should not - # decode the -expected.txt values either to allow comparisons. - text = self._expected_file_contents(test, '.txt', None) - if not text: + # FIXME: DRT output is actually utf-8, but since we don't decode the + # output from DRT (instead treating it as a binary string), we read the + # baselines as a binary string, too. + path = self.expected_filename(test, '.txt') + if not self.path_exists(path): return '' + text = self._filesystem.read_binary_file(path) return text.strip("\r\n").replace("\r\n", "\n") + "\n" def filename_to_uri(self, filename): @@ -362,7 +359,7 @@ class Port(object): protocol = "http" return "%s://127.0.0.1:%u/%s" % (protocol, port, relative_path) - return abspath_to_uri(os.path.abspath(filename)) + return path.abspath_to_uri(os.path.abspath(filename)) def tests(self, paths): """Return the list of tests found (relative to layout_tests_dir().""" @@ -373,20 +370,19 @@ class Port(object): Used by --clobber-old-results.""" layout_tests_dir = self.layout_tests_dir() - return filter(lambda x: os.path.isdir(os.path.join(layout_tests_dir, x)), - os.listdir(layout_tests_dir)) + return filter(lambda x: self._filesystem.isdir(self._filesystem.join(layout_tests_dir, x)), + self._filesystem.listdir(layout_tests_dir)) def path_isdir(self, path): - """Returns whether the path refers to a directory of tests. - - Used by test_expectations.py to apply rules to whole directories.""" - return os.path.isdir(path) + """Return True if the path refers to a directory of tests.""" + # Used by test_expectations.py to apply rules to whole directories. + return self._filesystem.isdir(path) def path_exists(self, path): - """Returns whether the path refers to an existing test or baseline.""" + """Return True if the path refers to an existing test or baseline.""" # Used by test_expectations.py to determine if an entry refers to a - # valid test and by printing.py to determine if baselines exist.""" - return os.path.exists(path) + # valid test and by printing.py to determine if baselines exist. + return self._filesystem.exists(path) def update_baseline(self, path, data, encoding): """Updates the baseline for a test. @@ -398,11 +394,12 @@ class Port(object): data: contents of the baseline. encoding: file encoding to use for the baseline. """ - write_mode = "w" + # FIXME: remove the encoding parameter in favor of text/binary + # functions. if encoding is None: - write_mode = "wb" - with codecs.open(path, write_mode, encoding=encoding) as file: - file.write(data) + self._filesystem.write_binary_file(path, data) + else: + self._filesystem.write_text_file(path, data) def uri_to_test_name(self, uri): """Return the base layout test name for a given URI. @@ -414,7 +411,7 @@ class Port(object): """ test = uri if uri.startswith("file:///"): - prefix = abspath_to_uri(self.layout_tests_dir()) + "/" + prefix = path.abspath_to_uri(self.layout_tests_dir()) + "/" return test[len(prefix):] if uri.startswith("http://127.0.0.1:8880/"): @@ -441,18 +438,16 @@ class Port(object): for test_or_category in self.skipped_layout_tests(): if test_or_category == test_name: return True - category = os.path.join(self.layout_tests_dir(), test_or_category) - if os.path.isdir(category) and test_name.startswith(test_or_category): + category = self._filesystem.join(self.layout_tests_dir(), + test_or_category) + if (self._filesystem.isdir(category) and + test_name.startswith(test_or_category)): return True return False def maybe_make_directory(self, *path): """Creates the specified directory if it doesn't already exist.""" - try: - os.makedirs(os.path.join(*path)) - except OSError, e: - if e.errno != errno.EEXIST: - raise + self._filesystem.maybe_make_directory(*path) def name(self): """Return the name of the port (e.g., 'mac', 'chromium-win-xp'). @@ -473,19 +468,13 @@ class Port(object): if not hasattr(self._options, name): return setattr(self._options, name, default_value) - # FIXME: This could be replaced by functions in webkitpy.common.checkout.scm. def path_from_webkit_base(self, *comps): """Returns the full path to path made by joining the top of the WebKit source tree and the list of path components in |*comps|.""" - if not self._webkit_base_dir: - abspath = os.path.abspath(__file__) - self._webkit_base_dir = abspath[0:abspath.find('WebKitTools')] - - return os.path.join(self._webkit_base_dir, *comps) + return self._config.path_from_webkit_base(*comps) - # FIXME: Callers should eventually move to scm.script_path. def script_path(self, script_name): - return self.path_from_webkit_base("WebKitTools", "Scripts", script_name) + return self._config.script_path(script_name) def path_to_test_expectations_file(self): """Update the test expectations to the passed-in string. @@ -726,50 +715,8 @@ class Port(object): e.message_with_output())) return self._pretty_patch_error_html - def _webkit_build_directory_command(self, args): - return ["perl", self.script_path("webkit-build-directory")] + args - - @memoized - def _webkit_top_level_build_directory(self, top_level=True): - """This directory is above where products are built to and contains things like the Configuration file.""" - args = self._webkit_build_directory_command(["--top-level"]) - return self._executive.run_command(args).rstrip() - - @memoized - def _webkit_configuration_build_directory(self, configuration=None): - """This is where products are normally built to.""" - if not configuration: - configuration = self.flag_from_configuration(self.get_option('configuration')) - args = self._webkit_build_directory_command(["--configuration", configuration]) - return self._executive.run_command(args).rstrip() - - def _configuration_file_path(self): - return os.path.join(self._webkit_top_level_build_directory(), "Configuration") - - # Easy override for unit tests - def _open_configuration_file(self): - configuration_path = self._configuration_file_path() - return codecs.open(configuration_path, "r", "utf-8") - - def _read_configuration(self): - try: - with self._open_configuration_file() as file: - return file.readline().rstrip() - except: - return None - - # FIXME: This list may be incomplete as Apple has some sekret configs. - _RECOGNIZED_CONFIGURATIONS = ("Debug", "Release") - def default_configuration(self): - # FIXME: Unify this with webkitdir.pm configuration reading code. - configuration = self._read_configuration() - if not configuration: - configuration = "Release" - if configuration not in self._RECOGNIZED_CONFIGURATIONS: - _log.warn("Configuration \"%s\" found in %s is not a recognized value.\n" % (configuration, self._configuration_file_path())) - _log.warn("Scripts may fail. See 'set-webkit-configuration --help'.") - return configuration + return self._config.default_configuration() # # PROTECTED ROUTINES @@ -777,6 +724,8 @@ class Port(object): # The routines below should only be called by routines in this class # or any of its subclasses. # + def _webkit_build_directory(self, args): + return self._config.build_directory(args[0]) def _path_to_apache(self): """Returns the full path to the apache binary. @@ -848,8 +797,8 @@ class Port(object): def _webkit_baseline_path(self, platform): """Return the full path to the top of the baseline tree for a given platform.""" - return os.path.join(self.layout_tests_dir(), 'platform', - platform) + return self._filesystem.join(self.layout_tests_dir(), 'platform', + platform) class Driver: @@ -883,16 +832,8 @@ class Driver: checksum - if present, the expected checksum for the image for this test - Returns a tuple of the following: - crash - a boolean indicating whether the driver crashed on the test - timeout - a boolean indicating whehter the test timed out - checksum - a string containing the checksum of the image, if - present - output - any text output - error - any unexpected or additional (or error) text output - - Note that the image itself should be written to the path that was - specified in the __init__() call.""" + Returns a TestOutput object. + """ raise NotImplementedError('Driver.run_test') # FIXME: This is static so we can test it w/o creating a Base instance. diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py index ee868e8..1e9c2b7 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py @@ -28,49 +28,21 @@ import optparse import os -import StringIO import sys import tempfile import unittest -from webkitpy.common.system.path import abspath_to_uri from webkitpy.common.system.executive import Executive, ScriptError +from webkitpy.common.system import executive_mock +from webkitpy.common.system import filesystem +from webkitpy.common.system import outputcapture +from webkitpy.common.system.path import abspath_to_uri from webkitpy.thirdparty.mock import Mock from webkitpy.tool import mocktool import base - -# FIXME: This makes StringIO objects work with "with". Remove -# when we upgrade to 2.6. -class NewStringIO(StringIO.StringIO): - def __enter__(self): - return self - - def __exit__(self, type, value, traceback): - pass - - -class MockExecutive(): - def __init__(self, exception): - self._exception = exception - - def run_command(self, *args, **kwargs): - raise self._exception - - -class UnitTestPort(base.Port): - """Subclass of base.Port used for unit testing.""" - def __init__(self, configuration_contents=None, configuration_exception=IOError, executive_exception=None): - base.Port.__init__(self) - self._configuration_contents = configuration_contents - self._configuration_exception = configuration_exception - if executive_exception: - self._executive = MockExecutive(executive_exception) - - def _open_configuration_file(self): - if self._configuration_contents: - return NewStringIO(self._configuration_contents) - raise self._configuration_exception +import config +import config_mock class PortTest(unittest.TestCase): @@ -102,18 +74,21 @@ class PortTest(unittest.TestCase): return new_file def test_pretty_patch_os_error(self): - port = UnitTestPort(executive_exception=OSError) + port = base.Port(executive=executive_mock.MockExecutive2(exception=OSError)) + oc = outputcapture.OutputCapture() + oc.capture_output() self.assertEqual(port.pretty_patch_text("patch.txt"), port._pretty_patch_error_html) # This tests repeated calls to make sure we cache the result. self.assertEqual(port.pretty_patch_text("patch.txt"), port._pretty_patch_error_html) + oc.restore_output() def test_pretty_patch_script_error(self): # FIXME: This is some ugly white-box test hacking ... base._pretty_patch_available = True - port = UnitTestPort(executive_exception=ScriptError) + port = base.Port(executive=executive_mock.MockExecutive2(exception=ScriptError)) self.assertEqual(port.pretty_patch_text("patch.txt"), port._pretty_patch_error_html) @@ -194,13 +169,9 @@ class PortTest(unittest.TestCase): self.assertFalse('nosuchthing' in diff) def test_default_configuration_notfound(self): - # Regular IOError thrown while trying to get the configuration. - port = UnitTestPort() - self.assertEqual(port.default_configuration(), "Release") - - # More exotic OSError thrown. - port = UnitTestPort(configuration_exception=OSError) - self.assertEqual(port.default_configuration(), "Release") + # Test that we delegate to the config object properly. + port = base.Port(config=config_mock.MockConfig(default_configuration='default')) + self.assertEqual(port.default_configuration(), 'default') def test_layout_tests_skipping(self): port = base.Port() @@ -209,14 +180,6 @@ class PortTest(unittest.TestCase): self.assertTrue(port.skips_layout_test('media/video-zoom.html')) self.assertFalse(port.skips_layout_test('foo/foo.html')) - def test_default_configuration_found(self): - port = UnitTestPort(configuration_contents="Debug") - self.assertEqual(port.default_configuration(), "Debug") - - def test_default_configuration_unknown(self): - port = UnitTestPort(configuration_contents="weird_value") - self.assertEqual(port.default_configuration(), "weird_value") - def test_setup_test_run(self): port = base.Port() # This routine is a no-op. We just test it for coverage. @@ -229,7 +192,6 @@ class PortTest(unittest.TestCase): self.assertTrue('css2.1' in dirs) def test_filename_to_uri(self): - port = base.Port() layout_test_dir = port.layout_tests_dir() test_file = os.path.join(layout_test_dir, "foo", "bar.html") diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py index f93f9a8..3149290 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py @@ -46,13 +46,11 @@ import webbrowser from webkitpy.common.system.executive import Executive from webkitpy.common.system.path import cygpath from webkitpy.layout_tests.layout_package import test_expectations +from webkitpy.layout_tests.layout_package import test_output import base import http_server -from webkitpy.common.system.executive import Executive -from webkitpy.layout_tests.layout_package import test_expectations - # Chromium DRT on OSX uses WebKitDriver. if sys.platform == 'darwin': import webkit @@ -447,6 +445,15 @@ class ChromiumDriver(base.Driver): cmd += "\n" return cmd + def _output_image(self): + """Returns the image output which driver generated.""" + png_path = self._image_path + if png_path and os.path.isfile(png_path): + with open(png_path, 'rb') as image_file: + return image_file.read() + else: + return None + def run_test(self, uri, timeoutms, checksum): output = [] error = [] @@ -498,8 +505,9 @@ class ChromiumDriver(base.Driver): (line, crash) = self._write_command_and_read_line(input=None) - return (crash, timeout, actual_checksum, ''.join(output), - ''.join(error)) + return test_output.TestOutput( + ''.join(output), self._output_image(), actual_checksum, + crash, time.time() - start_time, timeout, ''.join(error)) def stop(self): if self._proc: diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_gpu.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_gpu.py index 57b6989..54a0fee 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_gpu.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_gpu.py @@ -69,11 +69,14 @@ def _set_gpu_options(options): options.accelerated_compositing = True if options.accelerated_2d_canvas is None: options.accelerated_2d_canvas = True - if options.builder_name is not None: - options.builder_name = options.builder_name + ' - GPU' if options.use_drt is None: options.use_drt = True + # FIXME: Remove this after http://codereview.chromium.org/5133001/ is enabled + # on the bots. + if options.builder_name is not None and not ' - GPU' in options.builder_name: + options.builder_name = options.builder_name + ' - GPU' + def _gpu_overrides(port): try: diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/config.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/config.py index 2364098..cad5e37 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/config.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/config.py @@ -32,28 +32,29 @@ # FIXME: This file needs to be unified with common/checkout/scm.py and # common/config/ports.py . -from __future__ import with_statement - -import codecs import os -from webkitpy.common.checkout import scm from webkitpy.common.system import logutils +from webkitpy.common.system import executive _log = logutils.get_logger(__file__) # -# This is used to record if we've already hit the filesystem to look +# FIXME: This is used to record if we've already hit the filesystem to look # for a default configuration. We cache this to speed up the unit tests, -# but this can be reset with clear_cached_configuration(). +# but this can be reset with clear_cached_configuration(). This should be +# replaced with us consistently using MockConfigs() for tests that don't +# hit the filesystem at all and provide a reliable value. # -_determined_configuration = None +_have_determined_configuration = False +_configuration = "Release" def clear_cached_configuration(): - global _determined_configuration - _determined_configuration = -1 + global _have_determined_configuration, _configuration + _have_determined_configuration = False + _configuration = "Release" class Config(object): @@ -62,8 +63,9 @@ class Config(object): "Release": "--release", } - def __init__(self, executive): + def __init__(self, executive, filesystem): self._executive = executive + self._filesystem = filesystem self._webkit_base_dir = None self._default_configuration = None self._build_directories = {} @@ -115,40 +117,54 @@ class Config(object): return self._default_configuration def path_from_webkit_base(self, *comps): - return os.path.join(self.webkit_base_dir(), *comps) + return self._filesystem.join(self.webkit_base_dir(), *comps) def webkit_base_dir(self): """Returns the absolute path to the top of the WebKit tree. Raises an AssertionError if the top dir can't be determined.""" - # FIXME: Consider determining this independently of scm in order - # to be able to run in a bare tree. + # Note: this code somewhat duplicates the code in + # scm.find_checkout_root(). However, that code only works if the top + # of the SCM repository also matches the top of the WebKit tree. The + # Chromium ports, for example, only check out subdirectories like + # WebKitTools/Scripts, and so we still have to do additional work + # to find the top of the tree. + # + # This code will also work if there is no SCM system at all. if not self._webkit_base_dir: - self._webkit_base_dir = scm.find_checkout_root() - assert self._webkit_base_dir, "Could not determine the top of the WebKit checkout" + abspath = os.path.abspath(__file__) + self._webkit_base_dir = abspath[0:abspath.find('WebKitTools')] return self._webkit_base_dir def _script_path(self, script_name): - return os.path.join(self.webkit_base_dir(), "WebKitTools", - "Scripts", script_name) + return self._filesystem.join(self.webkit_base_dir(), "WebKitTools", + "Scripts", script_name) def _determine_configuration(self): # This mirrors the logic in webkitdirs.pm:determineConfiguration(). - global _determined_configuration - if _determined_configuration == -1: + # + # FIXME: See the comment at the top of the file regarding unit tests + # and our use of global mutable static variables. + global _have_determined_configuration, _configuration + if not _have_determined_configuration: contents = self._read_configuration() + if not contents: + contents = "Release" if contents == "Deployment": contents = "Release" if contents == "Development": contents = "Debug" - _determined_configuration = contents - return _determined_configuration + _configuration = contents + _have_determined_configuration = True + return _configuration def _read_configuration(self): - configuration_path = os.path.join(self.build_directory(None), - "Configuration") - if not os.path.exists(configuration_path): + try: + configuration_path = self._filesystem.join(self.build_directory(None), + "Configuration") + if not self._filesystem.exists(configuration_path): + return None + except (OSError, executive.ScriptError): return None - with codecs.open(configuration_path, "r", "utf-8") as fh: - return fh.read().rstrip() + return self._filesystem.read_text_file(configuration_path).rstrip() diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/config_mock.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/config_mock.py new file mode 100644 index 0000000..af71fa3 --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/config_mock.py @@ -0,0 +1,50 @@ +#!/usr/bin/env python +# Copyright (C) 2010 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. + +"""Wrapper objects for WebKit-specific utility routines.""" + + +class MockConfig(object): + def __init__(self, default_configuration='Release'): + self._default_configuration = default_configuration + + def build_directory(self, configuration): + return "/build" + + def build_dumprendertree(self, configuration): + return True + + def default_configuration(self): + return self._default_configuration + + def path_from_webkit_base(self, *comps): + return "/" + "/".join(list(comps)) + + def webkit_base_dir(self): + return "/" diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/config_standalone.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/config_standalone.py new file mode 100644 index 0000000..3dec3b9 --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/config_standalone.py @@ -0,0 +1,70 @@ +# Copyright (C) 2010 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. + +"""FIXME: This script is used by +config_unittest.test_default_configuration__standalone() to read the +default configuration to work around any possible caching / reset bugs. See +https://bugs.webkit.org/show_bug?id=49360 for the motivation. We can remove +this test when we remove the global configuration cache in config.py.""" + +import os +import unittest +import sys + + +# Ensure that webkitpy is in PYTHONPATH. +this_dir = os.path.abspath(sys.path[0]) +up = os.path.dirname +script_dir = up(up(up(this_dir))) +if script_dir not in sys.path: + sys.path.append(script_dir) + +from webkitpy.common.system import executive +from webkitpy.common.system import executive_mock +from webkitpy.common.system import filesystem +from webkitpy.common.system import filesystem_mock + +import config + + +def main(argv=None): + if not argv: + argv = sys.argv + + if len(argv) == 3 and argv[1] == '--mock': + e = executive_mock.MockExecutive2(output='foo') + fs = filesystem_mock.MockFileSystem({'foo/Configuration': argv[2]}) + else: + e = executive.Executive() + fs = filesystem.FileSystem() + + c = config.Config(e, fs) + print c.default_configuration() + +if __name__ == '__main__': + main() diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/config_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/config_unittest.py index 4674cba..9bea014 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/config_unittest.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/config_unittest.py @@ -26,150 +26,157 @@ # (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 codecs import os -import StringIO +import sys import unittest +from webkitpy.common.system import executive +from webkitpy.common.system import executive_mock +from webkitpy.common.system import filesystem +from webkitpy.common.system import filesystem_mock from webkitpy.common.system import outputcapture import config - -# FIXME: This makes StringIO objects work with "with". Remove -# when we upgrade to 2.6. -class NewStringIO(StringIO.StringIO): - def __enter__(self): - return self - - def __exit__(self, type, value, traceback): - pass - - -class MockExecutive(object): - def __init__(self, output='', exit_code=0): - self._output = output - self._exit_code = exit_code - - def run_command(self, arg_list, return_exit_code=False, - decode_output=False): - if return_exit_code: - return self._exit_code - return self._output - - class ConfigTest(unittest.TestCase): def tearDown(self): config.clear_cached_configuration() - def assertConfiguration(self, contents, expected): - # This tests that a configuration file containing - # _contents_ endsd up being interpreted as _expected_. - # - # FIXME: rewrite this when we have a filesystem abstraction we - # can mock out more easily. - config.clear_cached_configuration() - orig_open = codecs.open - - def wrap_open(contents): - def open_wrapper(path, mode, encoding): - return NewStringIO(contents) - return open_wrapper + def make_config(self, output='', files={}, exit_code=0, exception=None): + e = executive_mock.MockExecutive2(output=output, exit_code=exit_code, + exception=exception) + fs = filesystem_mock.MockFileSystem(files) + return config.Config(e, fs) - try: - orig_exists = os.path.exists - os.path.exists = lambda p: True - codecs.open = wrap_open(contents) - - e = MockExecutive(output='foo') - c = config.Config(e) - self.assertEqual(c.default_configuration(), expected) - finally: - os.path.exists = orig_exists - codecs.open = orig_open + def assert_configuration(self, contents, expected): + # This tests that a configuration file containing + # _contents_ ends up being interpreted as _expected_. + c = self.make_config('foo', {'foo/Configuration': contents}) + self.assertEqual(c.default_configuration(), expected) def test_build_directory_toplevel(self): - e = MockExecutive(output="toplevel") - c = config.Config(e) + c = self.make_config('toplevel') self.assertEqual(c.build_directory(None), 'toplevel') # Test again to check caching self.assertEqual(c.build_directory(None), 'toplevel') def test_build_directory__release(self): - e = MockExecutive(output="release") - c = config.Config(e) + c = self.make_config('release') self.assertEqual(c.build_directory('Release'), 'release') def test_build_directory__debug(self): - e = MockExecutive(output="debug") - c = config.Config(e) + c = self.make_config('debug') self.assertEqual(c.build_directory('Debug'), 'debug') def test_build_directory__unknown(self): - e = MockExecutive(output="unknown") - c = config.Config(e) + c = self.make_config("unknown") self.assertRaises(KeyError, c.build_directory, 'Unknown') def test_build_dumprendertree__success(self): - e = MockExecutive(exit_code=0) - c = config.Config(e) + c = self.make_config(exit_code=0) self.assertTrue(c.build_dumprendertree("Debug")) self.assertTrue(c.build_dumprendertree("Release")) self.assertRaises(KeyError, c.build_dumprendertree, "Unknown") def test_build_dumprendertree__failure(self): - e = MockExecutive(exit_code=-1) - c = config.Config(e) + c = self.make_config(exit_code=-1) + # FIXME: Build failures should log errors. However, the message we + # get depends on how we're being called; as a standalone test, + # we'll get the "no handlers found" message. As part of + # test-webkitpy, we get the actual message. Really, we need + # outputcapture to install its own handler. oc = outputcapture.OutputCapture() oc.capture_output() self.assertFalse(c.build_dumprendertree('Debug')) - (out, err) = oc.restore_output() + oc.restore_output() oc.capture_output() self.assertFalse(c.build_dumprendertree('Release')) oc.restore_output() def test_default_configuration__release(self): - self.assertConfiguration('Release', 'Release') + self.assert_configuration('Release', 'Release') def test_default_configuration__debug(self): - self.assertConfiguration('Debug', 'Debug') + self.assert_configuration('Debug', 'Debug') def test_default_configuration__deployment(self): - self.assertConfiguration('Deployment', 'Release') + self.assert_configuration('Deployment', 'Release') def test_default_configuration__development(self): - self.assertConfiguration('Development', 'Debug') + self.assert_configuration('Development', 'Debug') def test_default_configuration__notfound(self): # This tests what happens if the default configuration file # doesn't exist. - config.clear_cached_configuration() - try: - orig_exists = os.path.exists - os.path.exists = lambda p: False - e = MockExecutive(output="foo") - c = config.Config(e) - self.assertEqual(c.default_configuration(), "Release") - finally: - os.path.exists = orig_exists + c = self.make_config(output='foo', files={'foo/Configuration': None}) + self.assertEqual(c.default_configuration(), "Release") def test_default_configuration__unknown(self): # Ignore the warning about an unknown configuration value. oc = outputcapture.OutputCapture() oc.capture_output() - self.assertConfiguration('Unknown', 'Unknown') + self.assert_configuration('Unknown', 'Unknown') oc.restore_output() - def test_path_from_webkit_base(self, *comps): - c = config.Config(None) + def test_default_configuration__standalone(self): + # FIXME: This test runs a standalone python script to test + # reading the default configuration to work around any possible + # caching / reset bugs. See https://bugs.webkit.org/show_bug?id=49360 + # for the motivation. We can remove this test when we remove the + # global configuration cache in config.py. + e = executive.Executive() + fs = filesystem.FileSystem() + c = config.Config(e, fs) + script = c.path_from_webkit_base('WebKitTools', 'Scripts', + 'webkitpy', 'layout_tests', 'port', 'config_standalone.py') + + # Note: don't use 'Release' here, since that's the normal default. + expected = 'Debug' + + args = [sys.executable, script, '--mock', expected] + actual = e.run_command(args).rstrip() + self.assertEqual(actual, expected) + + def test_default_configuration__no_perl(self): + # We need perl to run webkit-build-directory to find out where the + # default configuration file is. See what happens if perl isn't + # installed. (We should get the default value, 'Release'). + c = self.make_config(exception=OSError) + actual = c.default_configuration() + self.assertEqual(actual, 'Release') + + def test_default_configuration__scripterror(self): + # We run webkit-build-directory to find out where the default + # configuration file is. See what happens if that script fails. + # (We should get the default value, 'Release'). + c = self.make_config(exception=executive.ScriptError()) + actual = c.default_configuration() + self.assertEqual(actual, 'Release') + + def test_path_from_webkit_base(self): + # FIXME: We use a real filesystem here. Should this move to a + # mocked one? + c = config.Config(executive.Executive(), filesystem.FileSystem()) self.assertTrue(c.path_from_webkit_base('foo')) def test_webkit_base_dir(self): - c = config.Config(None) - self.assertTrue(c.webkit_base_dir()) + # FIXME: We use a real filesystem here. Should this move to a + # mocked one? + c = config.Config(executive.Executive(), filesystem.FileSystem()) + base_dir = c.webkit_base_dir() + self.assertTrue(base_dir) + + orig_cwd = os.getcwd() + os.chdir(os.environ['HOME']) + c = config.Config(executive.Executive(), filesystem.FileSystem()) + try: + base_dir_2 = c.webkit_base_dir() + self.assertEqual(base_dir, base_dir_2) + finally: + os.chdir(orig_cwd) if __name__ == '__main__': diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/dryrun.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/dryrun.py index 8a6af56..96d0d55 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/dryrun.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/dryrun.py @@ -48,6 +48,9 @@ from __future__ import with_statement import os import sys +import time + +from webkitpy.layout_tests.layout_package import test_output import base import factory @@ -109,19 +112,19 @@ class DryrunDriver(base.Driver): return None def run_test(self, uri, timeoutms, image_hash): + start_time = time.time() test_name = self._port.uri_to_test_name(uri) path = os.path.join(self._port.layout_tests_dir(), test_name) text_output = self._port.expected_text(path) if image_hash is not None: image = self._port.expected_image(path) - if image and self._image_path: - with open(self._image_path, 'w') as f: - f.write(image) hash = self._port.expected_checksum(path) else: + image = None hash = None - return (False, False, hash, text_output, None) + return test_output.TestOutput(text_output, image, hash, False, + time.time() - start_time, False, None) def start(self): pass diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/google_chrome_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/google_chrome_unittest.py index c4c885d..e60c274 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/google_chrome_unittest.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/google_chrome_unittest.py @@ -28,7 +28,8 @@ import codecs import os import unittest -import base_unittest +from webkitpy.common import newstringio + import factory import google_chrome @@ -77,7 +78,7 @@ class GetGoogleChromePortTest(unittest.TestCase): def mock_open(path, mode, encoding): if 'test_expectations_chrome.txt' in path: - return base_unittest.NewStringIO(expected_string) + return newstringio.StringIO(expected_string) return orig_open(path, mode, encoding) try: diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/http_lock.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/http_lock.py index d65801d..8995b21 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/http_lock.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/http_lock.py @@ -36,6 +36,7 @@ import tempfile import time from webkitpy.common.system.executive import Executive +from webkitpy.common.system.file_lock import FileLock _log = logging.getLogger("webkitpy.layout_tests.port.http_lock") @@ -52,10 +53,9 @@ class HttpLock(object): self._lock_file_path_prefix = os.path.join(self._lock_path, self._lock_file_prefix) self._guard_lock_file = os.path.join(self._lock_path, guard_lock) + self._guard_lock = FileLock(self._guard_lock_file) self._process_lock_file_name = "" self._executive = Executive() - # maximum wait time for the lock creation - self._guard_lock_max_wait = 1 * 60 def cleanup_http_lock(self): """Delete the lock file if exists.""" @@ -95,35 +95,31 @@ class HttpLock(object): _log.debug("Removing stuck lock file: %s" % lock_list[0]) os.unlink(lock_list[0]) return - except IOError, OSError: + except (IOError, OSError): return return int(current_pid) def _create_lock_file(self): """The lock files are used to schedule the running test sessions in first - come first served order. The sequential guard lock ensures that the lock - numbers are sequential.""" + come first served order. The guard lock ensures that the lock numbers are + sequential.""" if not os.path.exists(self._lock_path): _log.debug("Lock directory does not exist: %s" % self._lock_path) return False - start_time = time.time() - while(True): - try: - sequential_guard_lock = os.open(self._guard_lock_file, os.O_CREAT | os.O_EXCL) - self._process_lock_file_name = (self._lock_file_path_prefix + - str(self._next_lock_number())) - lock_file = open(self._process_lock_file_name, 'w') - _log.debug("Creating lock file: %s" % self._process_lock_file_name) - lock_file.write(str(os.getpid())) - lock_file.close() - os.close(sequential_guard_lock) - os.unlink(self._guard_lock_file) - return True - except OSError: - if time.time() - start_time > self._guard_lock_max_wait: - _log.debug("Lock does not created: %s" % str(sys.exc_info())) - return False + if not self._guard_lock.acquire_lock(): + _log.debug("Guard lock timed out!") + return False + + self._process_lock_file_name = (self._lock_file_path_prefix + + str(self._next_lock_number())) + _log.debug("Creating lock file: %s" % self._process_lock_file_name) + lock_file = open(self._process_lock_file_name, 'w') + lock_file.write(str(os.getpid())) + lock_file.close() + self._guard_lock.release_lock() + return True + def wait_for_httpd_lock(self): """Create a lock file and wait until it's turn comes. If something goes wrong diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py index ff4086c..0a27821 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py @@ -36,6 +36,8 @@ import os import sys import time +from webkitpy.layout_tests.layout_package import test_output + import base @@ -289,6 +291,7 @@ class TestDriver(base.Driver): return True def run_test(self, uri, timeoutms, image_hash): + start_time = time.time() test_name = self._port.uri_to_test_name(uri) test = self._port._tests[test_name] if test.keyboard: @@ -297,13 +300,10 @@ class TestDriver(base.Driver): raise ValueError('exception from ' + test_name) if test.hang: time.sleep((float(timeoutms) * 4) / 1000.0) - - if self._port.get_option('pixel_tests') and test.actual_image: - with open(self._image_path, 'w') as file: - file.write(test.actual_image) - - return (test.crash, test.timeout, test.actual_checksum, - test.actual_text, test.error) + return test_output.TestOutput(test.actual_text, test.actual_image, + test.actual_checksum, test.crash, + time.time() - start_time, test.timeout, + test.error) def start(self): pass diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py index 0b324f5..06797c6 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py @@ -49,6 +49,7 @@ import shutil from webkitpy.common.system.executive import Executive import webkitpy.common.system.ospath as ospath +import webkitpy.layout_tests.layout_package.test_output as test_output import webkitpy.layout_tests.port.base as base import webkitpy.layout_tests.port.server_process as server_process @@ -81,14 +82,8 @@ class WebKitPort(base.Port): return '' def _build_driver(self): - exit_code = self._executive.run_command([ - self.script_path("build-dumprendertree"), - self.flag_from_configuration(self.get_option('configuration')), - ], return_exit_code=True) - if exit_code != 0: - _log.error("Failed to build DumpRenderTree") - return False - return True + configuration = self.get_option('configuration') + return self._config.build_dumprendertree(configuration) def _check_driver(self): driver_path = self._path_to_driver() @@ -357,8 +352,8 @@ class WebKitPort(base.Port): 'mac-tiger', 'mac-leopard', 'mac-snowleopard') def _build_path(self, *comps): - build_root = self._webkit_configuration_build_directory() - return os.path.join(build_root, *comps) + return self._filesystem.join(self._config.build_directory( + self.get_option('configuration')), *comps) def _path_to_driver(self): return self._build_path('DumpRenderTree') @@ -448,6 +443,7 @@ class WebKitDriver(base.Driver): command += "'" + image_hash command += "\n" + start_time = time.time() self._server_process.write(command) have_seen_content_type = False @@ -492,10 +488,6 @@ class WebKitDriver(base.Driver): timeout = deadline - time.time() line = self._server_process.read_line(timeout) - if self._image_path and len(self._image_path): - with open(self._image_path, "wb") as image_file: - image_file.write(image) - error_lines = self._server_process.error.splitlines() # FIXME: This is a hack. It is unclear why sometimes # we do not get any error lines from the server_process @@ -506,11 +498,11 @@ class WebKitDriver(base.Driver): # FIXME: This seems like the wrong section of code to be doing # this reset in. self._server_process.error = "" - return (self._server_process.crashed, - self._server_process.timed_out, - actual_image_hash, - output, - error) + return test_output.TestOutput(output, image, actual_image_hash, + self._server_process.crashed, + time.time() - start_time, + self._server_process.timed_out, + error) def stop(self): if self._server_process: diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py b/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py index f360adc..119de8c 100755 --- a/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py @@ -90,31 +90,18 @@ LAYOUT_TESTS_DIRECTORY = "LayoutTests" + os.sep TestExpectationsFile = test_expectations.TestExpectationsFile -class TestInfo: +class TestInput: """Groups information about a test for easy passing of data.""" - def __init__(self, port, filename, timeout): - """Generates the URI and stores the filename and timeout for this test. + def __init__(self, filename, timeout): + """Holds the input parameters for a test. Args: filename: Full path to the test. - timeout: Timeout for running the test in TestShell. + timeout: Timeout in msecs the driver should use while running the test """ + # FIXME: filename should really be test_name as a relative path. self.filename = filename - self._port = port - self.uri = port.filename_to_uri(filename) self.timeout = timeout - self._image_checksum = -1 - - def image_hash(self): - # Read the image_hash lazily to reduce startup time. - # This class is accessed across threads, but only one thread should - # ever be dealing with any given TestInfo so no locking is needed. - # - # Note that we use -1 to indicate that we haven't read the value, - # because expected_checksum() returns a string or None. - if self._image_checksum == -1: - self._image_checksum = self._port.expected_checksum(self.filename) - return self._image_checksum class ResultSummary(object): @@ -497,14 +484,13 @@ class TestRunner: return return_value - def _get_test_info_for_file(self, test_file): - """Returns the appropriate TestInfo object for the file. Mostly this + def _get_test_input_for_file(self, test_file): + """Returns the appropriate TestInput object for the file. Mostly this is used for looking up the timeout value (in ms) to use for the given test.""" if self._expectations.has_modifier(test_file, test_expectations.SLOW): - return TestInfo(self._port, test_file, - self._options.slow_time_out_ms) - return TestInfo(self._port, test_file, self._options.time_out_ms) + return TestInput(test_file, self._options.slow_time_out_ms) + return TestInput(test_file, self._options.time_out_ms) def _test_requires_lock(self, test_file): """Return True if the test needs to be locked when @@ -522,7 +508,7 @@ class TestRunner: cross-tests dependencies tend to occur within the same directory. Return: - The Queue of lists of TestInfo objects. + The Queue of lists of TestInput objects. """ test_lists = [] @@ -530,21 +516,21 @@ class TestRunner: if (self._options.experimental_fully_parallel or self._is_single_threaded()): for test_file in test_files: - test_info = self._get_test_info_for_file(test_file) + test_input = self._get_test_input_for_file(test_file) if self._test_requires_lock(test_file): - tests_to_http_lock.append(test_info) + tests_to_http_lock.append(test_input) else: - test_lists.append((".", [test_info])) + test_lists.append((".", [test_input])) else: tests_by_dir = {} for test_file in test_files: directory = self._get_dir_for_test_file(test_file) - test_info = self._get_test_info_for_file(test_file) + test_input = self._get_test_input_for_file(test_file) if self._test_requires_lock(test_file): - tests_to_http_lock.append(test_info) + tests_to_http_lock.append(test_input) else: tests_by_dir.setdefault(directory, []) - tests_by_dir[directory].append(test_info) + tests_by_dir[directory].append(test_input) # Sort by the number of tests in the dir so that the ones with the # most tests get run first in order to maximize parallelization. # Number of tests is a good enough, but not perfect, approximation @@ -808,12 +794,10 @@ class TestRunner: self._printer.print_unexpected_results(unexpected_results) if self._options.record_results: - # Write the same data to log files. - self._write_json_files(unexpected_results, result_summary, - individual_test_timings) - - # Upload generated JSON files to appengine server. - self._upload_json_files() + # Write the same data to log files and upload generated JSON files + # to appengine server. + self._upload_json_files(unexpected_results, result_summary, + individual_test_timings) # Write the summary to disk (results.html) and display it if requested. wrote_results = self._write_results_html_file(result_summary) @@ -892,10 +876,10 @@ class TestRunner: return failed_results - def _write_json_files(self, unexpected_results, result_summary, + def _upload_json_files(self, unexpected_results, result_summary, individual_test_timings): """Writes the results of the test run as JSON files into the results - dir. + dir and upload the files to the appengine server. There are three different files written into the results dir: unexpected_results.json: A short list of any unexpected results. @@ -924,50 +908,25 @@ class TestRunner: with codecs.open(expectations_path, "w", "utf-8") as file: file.write(u"ADD_EXPECTATIONS(%s);" % expectations_json) - json_layout_results_generator.JSONLayoutResultsGenerator( + generator = json_layout_results_generator.JSONLayoutResultsGenerator( self._port, self._options.builder_name, self._options.build_name, self._options.build_number, self._options.results_directory, BUILDER_BASE_URL, individual_test_timings, self._expectations, result_summary, self._test_files_list, not self._options.upload_full_results, - self._options.test_results_server) + self._options.test_results_server, + "layout-tests", + self._options.master_name) _log.debug("Finished writing JSON files.") - def _upload_json_files(self): - if not self._options.test_results_server: - return - - if not self._options.master_name: - _log.error("--test-results-server was set, but --master-name was not. Not uploading JSON files.") - return - - _log.info("Uploading JSON files for builder: %s", - self._options.builder_name) - - attrs = [("builder", self._options.builder_name), ("testtype", "layout-tests"), - ("master", self._options.master_name)] - json_files = ["expectations.json"] if self._options.upload_full_results: json_files.append("results.json") else: json_files.append("incremental_results.json") - files = [(file, os.path.join(self._options.results_directory, file)) - for file in json_files] - - uploader = test_results_uploader.TestResultsUploader( - self._options.test_results_server) - try: - # Set uploading timeout in case appengine server is having problem. - # 120 seconds are more than enough to upload test results. - uploader.upload(attrs, files, 120) - except Exception, err: - _log.error("Upload failed: %s" % err) - return - - _log.info("JSON files uploaded.") + generator.upload_json_files(json_files) def _print_config(self): """Prints the configuration for the test run.""" diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py index f21e7a5..54e1dc0 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py @@ -380,31 +380,33 @@ class RebaselineTest(unittest.TestCase): baseline = file + "-expected" + ext self.assertTrue(any(f.find(baseline) != -1 for f in file_list)) - # FIXME: This test is failing on the bots. Also, this test touches the - # file system. Unit tests should not read or write the file system. - # https://bugs.webkit.org/show_bug.cgi?id=47879 + # FIXME: Add tests to ensure that we're *not* writing baselines when we're not + # supposed to be. + def disabled_test_reset_results(self): + # FIXME: This test is disabled until we can rewrite it to use a + # mock filesystem. + # + # Test that we update expectations in place. If the expectation + # is missing, update the expected generic location. file_list = [] - original_open = codecs.open - try: - # Test that we update expectations in place. If the expectation - # is missing, update the expected generic location. - file_list = [] - codecs.open = _mocked_open(original_open, file_list) - passing_run(['--pixel-tests', - '--reset-results', - 'passes/image.html', - 'failures/expected/missing_image.html'], - tests_included=True) - self.assertEqual(len(file_list), 6) - self.assertBaselines(file_list, - "data/passes/image") - self.assertBaselines(file_list, - "data/failures/expected/missing_image") - finally: - codecs.open = original_open - - def test_new_baseline(self): + passing_run(['--pixel-tests', + '--reset-results', + 'passes/image.html', + 'failures/expected/missing_image.html'], + tests_included=True) + self.assertEqual(len(file_list), 6) + self.assertBaselines(file_list, + "data/passes/image") + self.assertBaselines(file_list, + "data/failures/expected/missing_image") + + def disabled_test_new_baseline(self): + # FIXME: This test is disabled until we can rewrite it to use a + # mock filesystem. + # + # Test that we update the platform expectations. If the expectation + # is mssing, then create a new expectation in the platform dir. file_list = [] original_open = codecs.open try: @@ -427,7 +429,7 @@ class RebaselineTest(unittest.TestCase): class TestRunnerWrapper(run_webkit_tests.TestRunner): - def _get_test_info_for_file(self, test_file): + def _get_test_input_for_file(self, test_file): return test_file diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/test_types/image_diff.py b/WebKitTools/Scripts/webkitpy/layout_tests/test_types/image_diff.py index 0b05802..41fe9bd 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/test_types/image_diff.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/test_types/image_diff.py @@ -54,116 +54,93 @@ _log = logging.getLogger("webkitpy.layout_tests.test_types.image_diff") class ImageDiff(test_type_base.TestTypeBase): - def _copy_output_png(self, test_filename, source_image, extension): - """Copies result files into the output directory with appropriate - names. - - Args: - test_filename: the test filename - source_file: path to the image file (either actual or expected) - extension: extension to indicate -actual.png or -expected.png - """ - self._make_output_directory(test_filename) - dest_image = self.output_filename(test_filename, extension) - - if os.path.exists(source_image): - shutil.copyfile(source_image, dest_image) - - def _save_baseline_files(self, filename, png_path, checksum, + def _save_baseline_files(self, filename, image, image_hash, generate_new_baseline): """Saves new baselines for the PNG and checksum. Args: filename: test filename - png_path: path to the actual PNG result file - checksum: value of the actual checksum result + image: a image output + image_hash: a checksum of the image generate_new_baseline: whether to generate a new, platform-specific baseline, or update the existing one """ - with open(png_path, "rb") as png_file: - png_data = png_file.read() - self._save_baseline_data(filename, png_data, ".png", encoding=None, + self._save_baseline_data(filename, image, ".png", encoding=None, generate_new_baseline=generate_new_baseline) - self._save_baseline_data(filename, checksum, ".checksum", + self._save_baseline_data(filename, image_hash, ".checksum", encoding="ascii", generate_new_baseline=generate_new_baseline) - def _create_image_diff(self, port, filename, configuration): + def _copy_image(self, filename, actual_image, expected_image): + self.write_output_files(filename, '.png', + output=actual_image, expected=expected_image, + encoding=None, print_text_diffs=False) + + def _copy_image_hash(self, filename, actual_image_hash, expected_image_hash): + self.write_output_files(filename, '.checksum', + actual_image_hash, expected_image_hash, + encoding="ascii", print_text_diffs=False) + + def _create_diff_image(self, port, filename, actual_image, expected_image): """Creates the visual diff of the expected/actual PNGs. - Args: - filename: the name of the test - configuration: Debug or Release - Returns True if the files are different, False if they match + Returns True if the images are different. """ diff_filename = self.output_filename(filename, - self.FILENAME_SUFFIX_COMPARE) - actual_filename = self.output_filename(filename, - self.FILENAME_SUFFIX_ACTUAL + '.png') - expected_filename = self.output_filename(filename, - self.FILENAME_SUFFIX_EXPECTED + '.png') + self.FILENAME_SUFFIX_COMPARE) + return port.diff_image(actual_image, expected_image, diff_filename) - expected_image = port.expected_image(filename) - with codecs.open(actual_filename, 'r+b', None) as file: - actual_image = file.read() - - result = port.diff_image(expected_image, actual_image, - diff_filename) - return result - - def compare_output(self, port, filename, output, test_args, configuration): + def compare_output(self, port, filename, test_args, actual_test_output, + expected_test_output): """Implementation of CompareOutput that checks the output image and checksum against the expected files from the LayoutTest directory. """ failures = [] # If we didn't produce a hash file, this test must be text-only. - if test_args.hash is None: + if actual_test_output.image_hash is None: return failures # If we're generating a new baseline, we pass. if test_args.new_baseline or test_args.reset_results: - self._save_baseline_files(filename, test_args.png_path, - test_args.hash, test_args.new_baseline) + self._save_baseline_files(filename, actual_test_output.image_hash, + actual_test_output.image, + test_args.new_baseline) return failures - # Compare hashes. - expected_hash = self._port.expected_checksum(filename) - expected_png = self._port.expected_image(filename) - - if not expected_png: + if not expected_test_output.image: # Report a missing expected PNG file. - self.write_output_files(filename, '.checksum', - test_args.hash, expected_hash, - encoding="ascii", - print_text_diffs=False) - self._copy_output_png(filename, test_args.png_path, '-actual.png') + self._copy_image(filename, actual_test_output.image, expected_image=None) + self._copy_image_hash(filename, actual_test_output.image_hash, + expected_test_output.image_hash) failures.append(test_failures.FailureMissingImage()) return failures - elif test_args.hash == expected_hash: + if not expected_test_output.image_hash: + # Report a missing expected checksum file. + self._copy_image(filename, actual_test_output.image, + expected_test_output.image) + self._copy_image_hash(filename, actual_test_output.image_hash, + expected_image_hash=None) + failures.append(test_failures.FailureMissingImageHash()) + return failures + + if actual_test_output.image_hash == expected_test_output.image_hash: # Hash matched (no diff needed, okay to return). return failures - self.write_output_files(filename, '.checksum', - test_args.hash, expected_hash, - encoding="ascii", - print_text_diffs=False) - - # FIXME: combine next two lines - self._copy_output_png(filename, test_args.png_path, '-actual.png') - self.write_output_files(filename, '.png', output=None, - expected=expected_png, - encoding=None, print_text_diffs=False) + self._copy_image(filename, actual_test_output.image, + expected_test_output.image) + self._copy_image_hash(filename, actual_test_output.image_hash, + expected_test_output.image_hash) # Even though we only use the result in one codepath below but we # still need to call CreateImageDiff for other codepaths. - images_are_different = self._create_image_diff(port, filename, configuration) - if not expected_hash: - failures.append(test_failures.FailureMissingImageHash()) - elif test_args.hash != expected_hash: - if images_are_different: - failures.append(test_failures.FailureImageHashMismatch()) - else: - failures.append(test_failures.FailureImageHashIncorrect()) + images_are_different = self._create_diff_image(port, filename, + actual_test_output.image, + expected_test_output.image) + if not images_are_different: + failures.append(test_failures.FailureImageHashIncorrect()) + else: + failures.append(test_failures.FailureImageHashMismatch()) return failures diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/test_types/test_type_base.py b/WebKitTools/Scripts/webkitpy/layout_tests/test_types/test_type_base.py index dcc64a3..4b96b3a 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/test_types/test_type_base.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/test_types/test_type_base.py @@ -140,18 +140,22 @@ class TestTypeBase(object): self._port.relative_test_filename(filename)) return os.path.splitext(output_filename)[0] + modifier - def compare_output(self, port, filename, output, test_args, configuration): + def compare_output(self, port, filename, test_args, actual_test_output, + expected_test_output): """Method that compares the output from the test with the expected value. This is an abstract method to be implemented by all sub classes. Args: + port: object implementing port-specific information and methods filename: absolute filename to test file - output: a string containing the output of the test test_args: a TestArguments object holding optional additional arguments - configuration: Debug or Release + actual_test_output: a TestOutput object which represents actual test + output + expected_test_output: a TestOutput object which represents a expected + test output Return: a list of TestFailure objects, empty if the test passes diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/test_types/text_diff.py b/WebKitTools/Scripts/webkitpy/layout_tests/test_types/text_diff.py index 4c32f0d..66e42ba 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/test_types/text_diff.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/test_types/text_diff.py @@ -55,13 +55,8 @@ class TestTextDiff(test_type_base.TestTypeBase): "\r\n", "\n") return norm + "\n" - def _get_normalized_expected_text(self, filename): - """Given the filename of the test, read the expected output from a file - and normalize the text. Returns a string with the expected text, or '' - if the expected output file was not found.""" - return self._port.expected_text(filename) - - def compare_output(self, port, filename, output, test_args, configuration): + def compare_output(self, port, filename, test_args, actual_test_output, + expected_test_output): """Implementation of CompareOutput that checks the output text against the expected text from the LayoutTest directory.""" failures = [] @@ -76,17 +71,18 @@ class TestTextDiff(test_type_base.TestTypeBase): return failures # Normalize text to diff - output = self._get_normalized_output_text(output) - expected = self._get_normalized_expected_text(filename) + actual_text = self._get_normalized_output_text(actual_test_output.text) + # Assuming expected_text is already normalized. + expected_text = expected_test_output.text # Write output files for new tests, too. - if port.compare_text(output, expected): + if port.compare_text(actual_text, expected_text): # Text doesn't match, write output files. - self.write_output_files(filename, ".txt", output, - expected, encoding=None, + self.write_output_files(filename, ".txt", actual_text, + expected_text, encoding=None, print_text_diffs=True) - if expected == '': + if expected_text == '': failures.append(test_failures.FailureMissingResult()) else: failures.append(test_failures.FailureTextMismatch()) diff --git a/WebKitTools/Scripts/webkitpy/style/checker.py b/WebKitTools/Scripts/webkitpy/style/checker.py index fb93eb9..e10eec5 100644 --- a/WebKitTools/Scripts/webkitpy/style/checker.py +++ b/WebKitTools/Scripts/webkitpy/style/checker.py @@ -131,11 +131,13 @@ _PATH_RULES_SPECIFIER = [ "WebKit/efl/ewk/", # There is no clean way to avoid "yy_*" names used by flex. "WebCore/css/CSSParser.cpp", - # There is no clean way to avoid "xxx_data" methods inside - # Qt's autotests since they are called automatically by the - # QtTest module. + # Qt code uses '_' in some places (such as private slots + # and on test xxx_data methos on tests) + "JavaScriptCore/qt/api/", + "WebKit/qt/Api/", "WebKit/qt/tests/", - "JavaScriptCore/qt/tests"], + "WebKit/qt/declarative/", + "WebKit/qt/examples/"], ["-readability/naming"]), ([# The GTK+ APIs use GTK+ naming style, which includes # lower-cased, underscore-separated values. @@ -232,15 +234,9 @@ _TEXT_FILE_EXTENSIONS = [ # WebKit maintains some files in Mozilla style on purpose to ease # future merges. _SKIPPED_FILES_WITH_WARNING = [ - # The Qt API and tests do not follow WebKit style. - # They follow Qt style. :) "gtk2drawing.c", # WebCore/platform/gtk/gtk2drawing.c "gtkdrawing.h", # WebCore/platform/gtk/gtkdrawing.h - "JavaScriptCore/qt/api/", "WebKit/gtk/tests/", - "WebKit/qt/Api/", - "WebKit/qt/tests/", - "WebKit/qt/examples/", # Soup API that is still being cooked, will be removed from WebKit # in a few months when it is merged into soup proper. The style # follows the libsoup style completely. diff --git a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py index 43d24fe..94d2c29 100755 --- a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py +++ b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py @@ -223,11 +223,29 @@ class GlobalVariablesTest(unittest.TestCase): "readability/naming") assertNoCheck("WebCore/css/CSSParser.cpp", "readability/naming") + + # Test if Qt exceptions are indeed working + assertCheck("JavaScriptCore/qt/api/qscriptengine.cpp", + "readability/braces") + assertCheck("WebKit/qt/Api/qwebpage.cpp", + "readability/braces") + assertCheck("WebKit/qt/tests/qwebelement/tst_qwebelement.cpp", + "readability/braces") + assertCheck("WebKit/qt/declarative/platformplugin/WebPlugin.cpp", + "readability/braces") + assertCheck("WebKit/qt/examples/platformplugin/WebPlugin.cpp", + "readability/braces") + assertNoCheck("JavaScriptCore/qt/api/qscriptengine.cpp", + "readability/naming") + assertNoCheck("WebKit/qt/Api/qwebpage.cpp", + "readability/naming") assertNoCheck("WebKit/qt/tests/qwebelement/tst_qwebelement.cpp", "readability/naming") - assertNoCheck( - "JavaScriptCore/qt/tests/qscriptengine/tst_qscriptengine.cpp", - "readability/naming") + assertNoCheck("WebKit/qt/declarative/platformplugin/WebPlugin.cpp", + "readability/naming") + assertNoCheck("WebKit/qt/examples/platformplugin/WebPlugin.cpp", + "readability/naming") + assertNoCheck("WebCore/ForwardingHeaders/debugger/Debugger.h", "build/header_guard") @@ -277,12 +295,9 @@ class CheckerDispatcherSkipTest(unittest.TestCase): paths_to_skip = [ "gtk2drawing.c", "gtkdrawing.h", - "JavaScriptCore/qt/api/qscriptengine_p.h", "WebCore/platform/gtk/gtk2drawing.c", "WebCore/platform/gtk/gtkdrawing.h", "WebKit/gtk/tests/testatk.c", - "WebKit/qt/Api/qwebpage.h", - "WebKit/qt/tests/qwebsecurityorigin/tst_qwebsecurityorigin.cpp", ] for path in paths_to_skip: diff --git a/WebKitTools/Scripts/webkitpy/style/checkers/cpp.py b/WebKitTools/Scripts/webkitpy/style/checkers/cpp.py index cd9e6ae..590bba9 100644 --- a/WebKitTools/Scripts/webkitpy/style/checkers/cpp.py +++ b/WebKitTools/Scripts/webkitpy/style/checkers/cpp.py @@ -152,6 +152,38 @@ def subn(pattern, replacement, s): return _regexp_compile_cache[pattern].subn(replacement, s) +def iteratively_replace_matches_with_char(pattern, char_replacement, s): + """Returns the string with replacement done. + + Every character in the match is replaced with char. + Due to the iterative nature, pattern should not match char or + there will be an infinite loop. + + Example: + pattern = r'<[^>]>' # template parameters + char_replacement = '_' + s = 'A<B<C, D>>' + Returns 'A_________' + + Args: + pattern: The regex to match. + char_replacement: The character to put in place of every + character of the match. + s: The string on which to do the replacements. + + Returns: + True, if the given line is blank. + """ + while True: + matched = search(pattern, s) + if not matched: + return s + start_match_index = matched.start(0) + end_match_index = matched.end(0) + match_length = end_match_index - start_match_index + s = s[:start_match_index] + char_replacement * match_length + s[end_match_index:] + + def up_to_unmatched_closing_paren(s): """Splits a string into two parts up to first unmatched ')'. @@ -284,20 +316,27 @@ class _FunctionState(object): self.current_function = '' self.in_a_function = False self.lines_in_function = 0 + # Make sure these will not be mistaken for real lines (even when a + # small amount is added to them). + self.body_start_line_number = -1000 + self.ending_line_number = -1000 - def begin(self, function_name): + def begin(self, function_name, body_start_line_number, ending_line_number): """Start analyzing function body. Args: function_name: The name of the function being tracked. + ending_line_number: The line number where the function ends. """ self.in_a_function = True self.lines_in_function = 0 self.current_function = function_name + self.body_start_line_number = body_start_line_number + self.ending_line_number = ending_line_number - def count(self): + def count(self, line_number): """Count line in current function body.""" - if self.in_a_function: + if self.in_a_function and line_number >= self.body_start_line_number: self.lines_in_function += 1 def check(self, error, line_number): @@ -325,7 +364,7 @@ class _FunctionState(object): self.current_function, self.lines_in_function, trigger)) def end(self): - """Stop analizing function body.""" + """Stop analyzing function body.""" self.in_a_function = False @@ -577,8 +616,8 @@ class CleansedLines(object): def close_expression(clean_lines, line_number, pos): """If input points to ( or { or [, finds the position that closes it. - If lines[line_number][pos] points to a '(' or '{' or '[', finds the the - line_number/pos that correspond to the closing of the expression. + If clean_lines.elided[line_number][pos] points to a '(' or '{' or '[', finds + the line_number/pos that correspond to the closing of the expression. Args: clean_lines: A CleansedLines instance containing the file. @@ -587,8 +626,8 @@ def close_expression(clean_lines, line_number, pos): Returns: A tuple (line, line_number, pos) pointer *past* the closing brace, or - (line, len(lines), -1) if we never find a close. Note we ignore - strings and comments when matching; and the line we return is the + ('', len(clean_lines.elided), -1) if we never find a close. Note we + ignore strings and comments when matching; and the line we return is the 'cleansed' line at line_number. """ @@ -604,8 +643,10 @@ def close_expression(clean_lines, line_number, pos): end_character = '}' num_open = line.count(start_character) - line.count(end_character) - while line_number < clean_lines.num_lines() and num_open > 0: + while num_open > 0: line_number += 1 + if line_number >= clean_lines.num_lines(): + return ('', len(clean_lines.elided), -1) line = clean_lines.elided[line_number] num_open += line.count(start_character) - line.count(end_character) # OK, now find the end_character that actually got us back to even @@ -1109,17 +1150,85 @@ def is_blank_line(line): return not line or line.isspace() +def detect_functions(clean_lines, line_number, function_state, error): + """Finds where functions start and end. + + Uses a simplistic algorithm assuming other style guidelines + (especially spacing) are followed. + Trivial bodies are unchecked, so constructors with huge initializer lists + may be missed. + + Args: + clean_lines: A CleansedLines instance containing the file. + line_number: The number of the line to check. + function_state: Current function name and lines in body so far. + error: The function to call with any errors found. + """ + # Are we now past the end of a function? + if function_state.ending_line_number + 1 == line_number: + function_state.end() + + # If we're in a function, don't try to detect a new one. + if function_state.in_a_function: + return + + lines = clean_lines.lines + line = lines[line_number] + raw = clean_lines.raw_lines + raw_line = raw[line_number] + + regexp = r'\s*(\w(\w|::|\*|\&|\s|<|>|,|~)*)\(' # decls * & space::name( ... + match_result = match(regexp, line) + if not match_result: + return + + # If the name is all caps and underscores, figure it's a macro and + # ignore it, unless it's TEST or TEST_F. + function_name = match_result.group(1).split()[-1] + if function_name != 'TEST' and function_name != 'TEST_F' and match(r'[A-Z_]+$', function_name): + return + + joined_line = '' + for start_line_number in xrange(line_number, clean_lines.num_lines()): + start_line = lines[start_line_number] + joined_line += ' ' + start_line.lstrip() + if search(r'(;|})', start_line): # Declarations and trivial functions + return # ... ignore + + if search(r'{', start_line): + # Replace template constructs with _ so that no spaces remain in the function name, + # while keeping the column numbers of other characters the same as "line". + line_with_no_templates = iteratively_replace_matches_with_char(r'<[^<>]*>', '_', line) + match_function = search(r'((\w|:|<|>|,|~)*)\(', line_with_no_templates) + if not match_function: + return # The '(' must have been inside of a template. + + # Use the column numbers from the modified line to find the + # function name in the original line. + function = line[match_function.start(1):match_function.end(1)] + + if match(r'TEST', function): # Handle TEST... macros + parameter_regexp = search(r'(\(.*\))', joined_line) + if parameter_regexp: # Ignore bad syntax + function += parameter_regexp.group(1) + else: + function += '()' + open_brace_index = start_line.find('{') + ending_line_number = close_expression(clean_lines, start_line_number, open_brace_index)[1] + function_state.begin(function, start_line_number + 1, ending_line_number) + return + + # No body for the function (or evidence of a non-function) was found. + error(line_number, 'readability/fn_size', 5, + 'Lint failed to find start of function body.') + + def check_for_function_lengths(clean_lines, line_number, function_state, error): """Reports for long function bodies. For an overview why this is done, see: http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Write_Short_Functions - Uses a simplistic algorithm assuming other style guidelines - (especially spacing) are followed. - Only checks unindented functions, so class members are unchecked. - Trivial bodies are unchecked, so constructors with huge initializer lists - may be missed. Blank/comment lines are not counted so as to avoid encouraging the removal of vertical space and commments just to get through a lint check. NOLINT *on the last line of a function* disables this check. @@ -1134,47 +1243,38 @@ def check_for_function_lengths(clean_lines, line_number, function_state, error): line = lines[line_number] raw = clean_lines.raw_lines raw_line = raw[line_number] - joined_line = '' - starting_func = False - regexp = r'(\w(\w|::|\*|\&|\s)*)\(' # decls * & space::name( ... - match_result = match(regexp, line) - if match_result: - # If the name is all caps and underscores, figure it's a macro and - # ignore it, unless it's TEST or TEST_F. - function_name = match_result.group(1).split()[-1] - if function_name == 'TEST' or function_name == 'TEST_F' or (not match(r'[A-Z_]+$', function_name)): - starting_func = True - - if starting_func: - body_found = False - for start_line_number in xrange(line_number, clean_lines.num_lines()): - start_line = lines[start_line_number] - joined_line += ' ' + start_line.lstrip() - if search(r'(;|})', start_line): # Declarations and trivial functions - body_found = True - break # ... ignore - if search(r'{', start_line): - body_found = True - function = search(r'((\w|:)*)\(', line).group(1) - if match(r'TEST', function): # Handle TEST... macros - parameter_regexp = search(r'(\(.*\))', joined_line) - if parameter_regexp: # Ignore bad syntax - function += parameter_regexp.group(1) - else: - function += '()' - function_state.begin(function) - break - if not body_found: - # No body for the function (or evidence of a non-function) was found. - error(line_number, 'readability/fn_size', 5, - 'Lint failed to find start of function body.') - elif match(r'^\}\s*$', line): # function end + if function_state.ending_line_number == line_number: # last line if not search(r'\bNOLINT\b', raw_line): function_state.check(error, line_number) - function_state.end() elif not match(r'^\s*$', line): - function_state.count() # Count non-blank/non-comment lines. + function_state.count(line_number) # Count non-blank/non-comment lines. + + +def check_pass_ptr_usage(clean_lines, line_number, function_state, error): + """Check for proper usage of Pass*Ptr. + + Currently this is limited to detecting declarations of Pass*Ptr + variables inside of functions. + + Args: + clean_lines: A CleansedLines instance containing the file. + line_number: The number of the line to check. + function_state: Current function name and lines in body so far. + error: The function to call with any errors found. + """ + if not function_state.in_a_function: + return + + lines = clean_lines.lines + line = lines[line_number] + if line_number >= function_state.body_start_line_number: + matched_pass_ptr = match(r'^\s*Pass([A-Z][A-Za-z]*)Ptr<', line) + if matched_pass_ptr: + type_name = 'Pass%sPtr' % matched_pass_ptr.group(1) + error(line_number, 'readability/pass_ptr', 5, + 'Local variables should never be %s (see ' + 'http://webkit.org/coding/RefPtr.html).' % type_name) def check_spacing(file_extension, clean_lines, line_number, error): @@ -2855,9 +2955,11 @@ def process_line(filename, file_extension, """ raw_lines = clean_lines.raw_lines + detect_functions(clean_lines, line, function_state, error) check_for_function_lengths(clean_lines, line, function_state, error) if search(r'\bNOLINT\b', raw_lines[line]): # ignore nolint lines return + check_pass_ptr_usage(clean_lines, line, function_state, error) check_for_multiline_comments_and_strings(clean_lines, line, error) check_style(clean_lines, line, file_extension, class_state, file_state, error) check_language(filename, clean_lines, line, file_extension, include_state, @@ -2942,6 +3044,7 @@ class CppChecker(object): 'readability/multiline_string', 'readability/naming', 'readability/null', + 'readability/pass_ptr', 'readability/streams', 'readability/todo', 'readability/utf8', diff --git a/WebKitTools/Scripts/webkitpy/style/checkers/cpp_unittest.py b/WebKitTools/Scripts/webkitpy/style/checkers/cpp_unittest.py index 6d5c24b..13b053c 100644 --- a/WebKitTools/Scripts/webkitpy/style/checkers/cpp_unittest.py +++ b/WebKitTools/Scripts/webkitpy/style/checkers/cpp_unittest.py @@ -205,10 +205,27 @@ class CppStyleTestBase(unittest.TestCase): cpp_style.remove_multi_line_comments(lines, error_collector) lines = cpp_style.CleansedLines(lines) for i in xrange(lines.num_lines()): + cpp_style.detect_functions(lines, i, + function_state, error_collector) cpp_style.check_for_function_lengths(lines, i, function_state, error_collector) return error_collector.results() + # Similar to perform_function_lengths_check, but calls check_pass_ptr_usage + # instead of check_for_function_lengths. + def perform_pass_ptr_check(self, code): + error_collector = ErrorCollector(self.assert_) + function_state = cpp_style._FunctionState(self.min_confidence) + lines = code.split('\n') + cpp_style.remove_multi_line_comments(lines, error_collector) + lines = cpp_style.CleansedLines(lines) + for i in xrange(lines.num_lines()): + cpp_style.detect_functions(lines, i, + function_state, error_collector) + cpp_style.check_pass_ptr_usage(lines, i, + function_state, error_collector) + return error_collector.results() + def perform_include_what_you_use(self, code, filename='foo.h', io=codecs): # First, build up the include state. error_collector = ErrorCollector(self.assert_) @@ -2432,6 +2449,20 @@ class CheckForFunctionLengthsTest(CppStyleTestBase): def test_function_length_check_definition_above_severity1(self): self.assert_function_length_check_above_error_level(1) + def test_function_length_check_definition_severity1_plus_indented(self): + error_level = 1 + error_lines = self.trigger_lines(error_level) + 1 + trigger_level = self.trigger_lines(self.min_confidence) + indent_spaces = ' ' + self.assert_function_lengths_check( + re.sub(r'(?m)^(.)', indent_spaces + r'\1', + 'void test_indent(int x)\n' + self.function_body(error_lines)), + ('Small and focused functions are preferred: ' + 'test_indent() has %d non-comment lines ' + '(error triggered by exceeding %d lines).' + ' [readability/fn_size] [%d]') + % (error_lines, trigger_level, error_level)) + def test_function_length_check_definition_severity1_plus_blanks(self): error_level = 1 error_lines = self.trigger_lines(error_level) + 1 @@ -2449,11 +2480,11 @@ class CheckForFunctionLengthsTest(CppStyleTestBase): error_lines = self.trigger_lines(error_level) + 1 trigger_level = self.trigger_lines(self.min_confidence) self.assert_function_lengths_check( - ('my_namespace::my_other_namespace::MyVeryLongTypeName*\n' - 'my_namespace::my_other_namespace::MyFunction(int arg1, char* arg2)' + ('my_namespace::my_other_namespace::MyVeryLongTypeName<Type1, bool func(const Element*)>*\n' + 'my_namespace::my_other_namespace<Type3, Type4>::~MyFunction<Type5<Type6, Type7> >(int arg1, char* arg2)' + self.function_body(error_lines)), ('Small and focused functions are preferred: ' - 'my_namespace::my_other_namespace::MyFunction()' + 'my_namespace::my_other_namespace<Type3, Type4>::~MyFunction<Type5<Type6, Type7> >()' ' has %d non-comment lines ' '(error triggered by exceeding %d lines).' ' [readability/fn_size] [%d]') @@ -2484,7 +2515,7 @@ class CheckForFunctionLengthsTest(CppStyleTestBase): 'FixGoogleUpdate_AllValues_MachineApp) has %d non-comment lines ' '(error triggered by exceeding %d lines).' ' [readability/fn_size] [%d]') - % (error_lines+1, trigger_level, error_level)) + % (error_lines, trigger_level, error_level)) def test_function_length_check_definition_severity1_for_bad_test_doesnt_break(self): error_level = 1 @@ -2714,6 +2745,65 @@ class NoNonVirtualDestructorsTest(CppStyleTestBase): 'virtual method(s), one declared at line 2. [runtime/virtual] [4]']) +class PassPtrTest(CppStyleTestBase): + # For http://webkit.org/coding/RefPtr.html + + def assert_pass_ptr_check(self, code, expected_message): + """Check warnings for Pass*Ptr are as expected. + + Args: + code: C++ source code expected to generate a warning message. + expected_message: Message expected to be generated by the C++ code. + """ + self.assertEquals(expected_message, + self.perform_pass_ptr_check(code)) + + def test_pass_ref_ptr_in_function(self): + # Local variables should never be PassRefPtr. + self.assert_pass_ptr_check( + 'int myFunction()\n' + '{\n' + ' PassRefPtr<Type1> variable = variable2;\n' + '}', + 'Local variables should never be PassRefPtr (see ' + 'http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5]') + + def test_pass_own_ptr_in_function(self): + # Local variables should never be PassRefPtr. + self.assert_pass_ptr_check( + 'int myFunction()\n' + '{\n' + ' PassOwnPtr<Type1> variable = variable2;\n' + '}', + 'Local variables should never be PassOwnPtr (see ' + 'http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5]') + + def test_pass_other_type_ptr_in_function(self): + # Local variables should never be PassRefPtr. + self.assert_pass_ptr_check( + 'int myFunction()\n' + '{\n' + ' PassOtherTypePtr<Type1> variable;\n' + '}', + 'Local variables should never be PassOtherTypePtr (see ' + 'http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5]') + + def test_pass_ref_ptr_return_value(self): + self.assert_pass_ptr_check( + 'PassRefPtr<Type1>\n' + 'myFunction(int)\n' + '{\n' + '}', + '') + + def test_pass_ref_ptr_parameter_value(self): + self.assert_pass_ptr_check( + 'int myFunction(PassRefPtr<Type1>)\n' + '{\n' + '}', + '') + + class WebKitStyleTest(CppStyleTestBase): # for http://webkit.org/coding/coding-style.html diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/feeders.py b/WebKitTools/Scripts/webkitpy/tool/bot/feeders.py index dc892a4..046c4c1 100644 --- a/WebKitTools/Scripts/webkitpy/tool/bot/feeders.py +++ b/WebKitTools/Scripts/webkitpy/tool/bot/feeders.py @@ -26,8 +26,8 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +from webkitpy.common.config.committervalidator import CommitterValidator from webkitpy.common.system.deprecated_logging import log -from webkitpy.common.net.bugzilla import CommitterValidator from webkitpy.tool.grammar import pluralize diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/__init__.py b/WebKitTools/Scripts/webkitpy/tool/commands/__init__.py index a070324..d2aa503 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/__init__.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/__init__.py @@ -1,6 +1,12 @@ # Required for Python to search this directory for module files +from webkitpy.tool.commands.download import * +from webkitpy.tool.commands.earlywarningsystem import * +from webkitpy.tool.commands.openbugs import OpenBugs from webkitpy.tool.commands.prettydiff import PrettyDiff +from webkitpy.tool.commands.queries import * +from webkitpy.tool.commands.queues import * from webkitpy.tool.commands.rebaseline import Rebaseline from webkitpy.tool.commands.rebaselineserver import RebaselineServer -# FIXME: Add the rest of the commands here. +from webkitpy.tool.commands.sheriffbot import * +from webkitpy.tool.commands.upload import * diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/index.html b/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/index.html index 5667cd2..8cc48c1 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/index.html +++ b/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/index.html @@ -1,11 +1,11 @@ <!DOCTYPE html> <!-- Copyright (c) 2010 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 @@ -15,7 +15,7 @@ * 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 @@ -32,9 +32,108 @@ <head> <title>Layout Test Rebaseline Server</title> <link rel="stylesheet" href="/main.css" type="text/css"> + <script src="/util.js"></script> + <script src="/loupe.js"></script> <script src="/main.js"></script> </head> <body class="loading"> + +<div id="header"> + <div id="controls"> + <!-- Add a dummy <select> node so that this lines up with the text on the left --> + <select style="visibility: hidden"></select> <a href="/quitquitquit">Exit</a> + </div> + + <span id="selectors"> + <label> + Failure type: + <select id="failure-type-selector"></select> + </label> + + <label> + Directory: + <select id="directory-selector"></select> + </label> + + <label> + Test: + <select id="test-selector"></select> + </label> + </span> + + <a id="test-link">View test</a> + + <span id="nav-buttons"> + <button id="previous-test">«</button> + <span id="test-index"></span> of <span id="test-count"></span> + <button id="next-test">»</button> + </span> +</div> + +<table id="test-output"> + <thead id="labels"> + <tr> + <th>Expected</th> + <th>Actual</th> + <th>Diff</th> + </tr> + </thead> + <tbody id="image-outputs" style="display: none"> + <tr> + <td colspan="3"><h2>Image</h2></td> + </tr> + <tr> + <td><img id="expected-image"></td> + <td><img id="actual-image"></td> + <td><canvas id="diff-canvas" width="800" height="600"></canvas></td> + </tr> + </tbody> + <tbody id="text-outputs" style="display: none"> + <tr> + <td colspan="3"><h2>Text</h2></td> + </tr> + <tr> + <td><pre id="expected-text"></pre></td> + <td><pre id="actual-text"></pre></td> + <td><pre id="diff-text"><pre></td> + </tr> + </tbody> +</table> + +<table id="loupe" style="display: none"> + <tr> + <td colspan="3" id="loupe-info"> + <span id="loupe-close" class="link">Close</span> + <label>Coordinate: <span id="loupe-coordinate"></span></label> + </td> + </tr> + <tr> + <td> + <div class="loupe-container"> + <canvas id="expected-loupe" width="210" height="210"></canvas> + <div class="center-highlight"></div> + </div> + </td> + <td> + <div class="loupe-container"> + <canvas id="actual-loupe" width="210" height="210"></canvas> + <div class="center-highlight"></div> + </div> + </td> + <td> + <div class="loupe-container"> + <canvas id="diff-loupe" width="210" height="210"></canvas> + <div class="center-highlight"></div> + </div> + </td> + </tr> + <tr id="loupe-colors"> + <td><label>Exp. color: <span id="expected-loupe-color"></span></label></td> + <td><label>Actual color: <span id="actual-loupe-color"></span></label></td> + <td><label>Diff color: <span id="diff-loupe-color"></span></label></td> + </tr> +</table> + </body> </html> diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/loupe.js b/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/loupe.js new file mode 100644 index 0000000..41f977a --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/loupe.js @@ -0,0 +1,144 @@ +/* + * Copyright (c) 2010 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. + */ + +var LOUPE_MAGNIFICATION_FACTOR = 10; + +function Loupe() +{ + this._node = $('loupe'); + this._currentCornerX = -1; + this._currentCornerY = -1; + + var self = this; + + function handleOutputClick(event) { self._handleOutputClick(event); } + $('expected-image').addEventListener('click', handleOutputClick); + $('actual-image').addEventListener('click', handleOutputClick); + $('diff-canvas').addEventListener('click', handleOutputClick); + + function handleLoupeClick(event) { self._handleLoupeClick(event); } + $('expected-loupe').addEventListener('click', handleLoupeClick); + $('actual-loupe').addEventListener('click', handleLoupeClick); + $('diff-loupe').addEventListener('click', handleLoupeClick); + + function hide(event) { self.hide(); } + $('loupe-close').addEventListener('click', hide); +} + +Loupe.prototype._handleOutputClick = function(event) +{ + // The -1 compensates for the border around the image/canvas. + this._showFor(event.offsetX - 1, event.offsetY - 1); +}; + +Loupe.prototype._handleLoupeClick = function(event) +{ + var deltaX = Math.floor(event.offsetX/LOUPE_MAGNIFICATION_FACTOR); + var deltaY = Math.floor(event.offsetY/LOUPE_MAGNIFICATION_FACTOR); + + this._showFor( + this._currentCornerX + deltaX, this._currentCornerY + deltaY); +} + +Loupe.prototype.hide = function() +{ + this._node.style.display = 'none'; +}; + +Loupe.prototype._showFor = function(x, y) +{ + this._fillFromImage(x, y, 'expected', $('expected-image')); + this._fillFromImage(x, y, 'actual', $('actual-image')); + this._fillFromCanvas(x, y, 'diff', $('diff-canvas')); + + this._node.style.display = ''; +}; + +Loupe.prototype._fillFromImage = function(x, y, type, sourceImage) +{ + var tempCanvas = document.createElement('canvas'); + tempCanvas.width = sourceImage.width; + tempCanvas.height = sourceImage.height; + var tempContext = tempCanvas.getContext('2d'); + + tempContext.drawImage(sourceImage, 0, 0); + + this._fillFromCanvas(x, y, type, tempCanvas); +}; + +Loupe.prototype._fillFromCanvas = function(x, y, type, canvas) +{ + var context = canvas.getContext('2d'); + var sourceImageData = + context.getImageData(0, 0, canvas.width, canvas.height); + + var targetCanvas = $(type + '-loupe'); + var targetContext = targetCanvas.getContext('2d'); + targetContext.fillStyle = 'rgba(255, 255, 255, 1)'; + targetContext.fillRect(0, 0, targetCanvas.width, targetCanvas.height); + + var sourceXOffset = (targetCanvas.width/LOUPE_MAGNIFICATION_FACTOR - 1)/2; + var sourceYOffset = (targetCanvas.height/LOUPE_MAGNIFICATION_FACTOR - 1)/2; + + function readPixelComponent(x, y, component) { + var offset = (y * sourceImageData.width + x) * 4 + component; + return sourceImageData.data[offset]; + } + + for (var i = -sourceXOffset; i <= sourceXOffset; i++) { + for (var j = -sourceYOffset; j <= sourceYOffset; j++) { + var sourceX = x + i; + var sourceY = y + j; + + var sourceR = readPixelComponent(sourceX, sourceY, 0); + var sourceG = readPixelComponent(sourceX, sourceY, 1); + var sourceB = readPixelComponent(sourceX, sourceY, 2); + var sourceA = readPixelComponent(sourceX, sourceY, 3)/255; + sourceA = Math.round(sourceA * 10)/10; + + var targetX = (i + sourceXOffset) * LOUPE_MAGNIFICATION_FACTOR; + var targetY = (j + sourceYOffset) * LOUPE_MAGNIFICATION_FACTOR; + var colorString = + sourceR + ', ' + sourceG + ', ' + sourceB + ', ' + sourceA; + targetContext.fillStyle = 'rgba(' + colorString + ')'; + targetContext.fillRect( + targetX, targetY, + LOUPE_MAGNIFICATION_FACTOR, LOUPE_MAGNIFICATION_FACTOR); + + if (i == 0 && j == 0) { + $('loupe-coordinate').textContent = sourceX + ', ' + sourceY; + $(type + '-loupe-color').textContent = colorString; + } + } + } + + this._currentCornerX = x - sourceXOffset; + this._currentCornerY = y - sourceYOffset; +}; diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.css b/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.css index 35bd6a5..6e90fe4 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.css +++ b/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.css @@ -46,7 +46,7 @@ div { a, .link { color: #aaf; text-decoration: underline; - cursor: pointer; + cursor: pointer; } .link.selected { @@ -54,3 +54,160 @@ a, .link { font-weight: bold; text-decoration: none; } + +#header { + padding: .5em 1em; + background: #333; + color: #fff; + -webkit-box-shadow: 0 1px 5px rgba(0, 0, 0, 0.5); + margin-bottom: 1em; +} + +#header label { + padding-right: 1em; + color: #ccc; +} + +#test-link { + margin-right: 1em; +} + +#header label span { + color: #fff; + font-weight: bold; +} + +#nav-buttons { + white-space: nowrap; +} + +#nav-buttons button { + background: #fff; + border: 0; + border-radius: 10px; +} + +#nav-buttons button:active { + -webkit-box-shadow: 0 0 5px #33f inset; + background: #aaa; +} + +#nav-buttons button[disabled] { + opacity: .5; +} + +#controls { + float: right; +} + +#test-output { + border-spacing: 0; + border-collapse: collapse; + margin: 0 auto; + width: 100%; +} + +#test-output td, +#test-output th { + padding: 0; + vertical-align: top; +} + +#image-outputs img, +#image-outputs canvas { + width: 800px; + height: 600px; + border: solid 1px #ddd; + -webkit-user-select: none; + -webkit-user-drag: none; + cursor: crosshair; +} + +#image-outputs img.loading, +#image-outputs canvas.loading { + opacity: .5; +} + +#image-outputs #actual-image { + margin: 0 1em; +} + +#test-output #labels th { + text-align: center; + color: #666; +} + +#text-outputs pre { + height: 600px; + width: 800px; + overflow: auto; +} + +#test-output h2 { + border-bottom: solid 1px #ccc; + font-weight: bold; + margin: 0; + background: #eee; +} + +#loupe { + -webkit-box-shadow: 2px 2px 5px rgba(0, 0, 0, .5); + position: absolute; + width: 634px; + top: 50%; + left: 50%; + margin-left: -151px; + margin-top: -50px; + background: #fff; + border-spacing: 0; + border-collapse: collapse; +} + +#loupe td { + padding: 0; + border: solid 1px #ccc; +} + +#loupe label { + color: #999; + padding-right: 1em; +} + +#loupe span { + color: #000; + font-weight: bold; +} + +#loupe canvas { + cursor: crosshair; +} + +#loupe #loupe-close { + float: right; +} + +#loupe #loupe-info { + background: #eee; + padding: .3em .5em; +} + +#loupe #loupe-colors td { + text-align: center; +} + +#loupe .loupe-container { + position: relative; + width: 210px; + height: 210px; +} + +#loupe .center-highlight { + position: absolute; + width: 10px; + height: 10px; + top: 50%; + left: 50%; + margin-left: -5px; + margin-top: -5px; + outline: solid 1px #999; +} diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js b/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js index 55f19a4..fa037b3 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js +++ b/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js @@ -28,9 +28,372 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +var ALL_DIRECTORY_PATH = '[all]'; + +var results; +var testsByFailureType = {}; +var testsByDirectory = {}; +var selectedTests = []; +var loupe; + function main() { + $('failure-type-selector').addEventListener('change', selectFailureType); + $('directory-selector').addEventListener('change', selectDirectory); + $('test-selector').addEventListener('change', selectTest); + $('next-test').addEventListener('click', nextTest); + $('previous-test').addEventListener('click', previousTest); + + loupe = new Loupe(); + + document.addEventListener('keydown', function(event) { + if (event.altKey || event.ctrlKey || event.metaKey || event.shiftKey) { + return; + } + + switch (event.keyIdentifier) { + case 'Left': + event.preventDefault(); + previousTest(); + break; + case 'Right': + event.preventDefault(); + nextTest(); + break; + } + }); + + loadText('/results.json', function(text) { + results = JSON.parse(text); + displayResults(); + }); +} + +/** + * Groups test results by failure type. + */ +function displayResults() +{ + var failureTypeSelector = $('failure-type-selector'); + var failureTypes = []; + + for (var testName in results.tests) { + var test = results.tests[testName]; + if (test.actual == 'PASS') { + continue; + } + var failureType = test.actual + ' (expected ' + test.expected + ')'; + if (!(failureType in testsByFailureType)) { + testsByFailureType[failureType] = []; + failureTypes.push(failureType); + } + testsByFailureType[failureType].push(testName); + } + + // Sort by number of failures + failureTypes.sort(function(a, b) { + return testsByFailureType[b].length - testsByFailureType[a].length; + }); + + for (var i = 0, failureType; failureType = failureTypes[i]; i++) { + var failureTypeOption = document.createElement('option'); + failureTypeOption.value = failureType; + failureTypeOption.textContent = failureType + ' - ' + testsByFailureType[failureType].length + ' tests'; + failureTypeSelector.appendChild(failureTypeOption); + } + + selectFailureType(); + document.body.classList.remove('loading'); } +/** + * For a given failure type, gets all the tests and groups them by directory + * (populating the directory selector with them). + */ +function selectFailureType() +{ + var selectedFailureType = getSelectValue('failure-type-selector'); + var tests = testsByFailureType[selectedFailureType]; + + testsByDirectory = {} + var displayDirectoryNamesByDirectory = {}; + var directories = []; + + // Include a special option for all tests + testsByDirectory[ALL_DIRECTORY_PATH] = tests; + displayDirectoryNamesByDirectory[ALL_DIRECTORY_PATH] = 'all'; + directories.push(ALL_DIRECTORY_PATH); + + // Roll up tests by ancestor directories + tests.forEach(function(test) { + var pathPieces = test.split('/'); + var pathDirectories = pathPieces.slice(0, pathPieces.length -1); + var ancestorDirectory = ''; + + pathDirectories.forEach(function(pathDirectory, index) { + ancestorDirectory += pathDirectory + '/'; + if (!(ancestorDirectory in testsByDirectory)) { + testsByDirectory[ancestorDirectory] = []; + var displayDirectoryName = new Array(index * 6).join(' ') + pathDirectory; + displayDirectoryNamesByDirectory[ancestorDirectory] = displayDirectoryName; + directories.push(ancestorDirectory); + } + + testsByDirectory[ancestorDirectory].push(test); + }); + }); + + directories.sort(); + + var directorySelector = $('directory-selector'); + directorySelector.innerHTML = ''; + + directories.forEach(function(directory) { + var directoryOption = document.createElement('option'); + directoryOption.value = directory; + directoryOption.innerHTML = + displayDirectoryNamesByDirectory[directory] + ' - ' + + testsByDirectory[directory].length + ' tests'; + directorySelector.appendChild(directoryOption); + }); + + selectDirectory(); +} + +/** + * For a given failure type and directory and failure type, gets all the tests + * in that directory and populatest the test selector with them. + */ +function selectDirectory() +{ + var selectedDirectory = getSelectValue('directory-selector'); + selectedTests = testsByDirectory[selectedDirectory]; + + selectedTests.sort(); + + var testSelector = $('test-selector'); + testSelector.innerHTML = ''; + + selectedTests.forEach(function(testName) { + var testOption = document.createElement('option'); + testOption.value = testName; + var testDisplayName = testName; + if (testName.lastIndexOf(selectedDirectory) == 0) { + testDisplayName = testName.substring(selectedDirectory.length); + } + testOption.innerHTML = ' ' + testDisplayName; + testSelector.appendChild(testOption); + }); + + selectTest(); +} + +function getSelectedTest() +{ + return getSelectValue('test-selector'); +} + +function selectTest() +{ + var selectedTest = getSelectedTest(); + + if (results.tests[selectedTest].actual.indexOf('IMAGE') != -1) { + $('image-outputs').style.display = ''; + displayImageResults(selectedTest); + } else { + $('image-outputs').style.display = 'none'; + } + + if (results.tests[selectedTest].actual.indexOf('TEXT') != -1) { + $('text-outputs').style.display = ''; + displayTextResults(selectedTest); + } else { + $('text-outputs').style.display = 'none'; + } + + updateState(); + loupe.hide(); +} + +function updateState() +{ + var testName = getSelectedTest(); + var testIndex = selectedTests.indexOf(testName); + var testCount = selectedTests.length + $('test-index').textContent = testIndex + 1; + $('test-count').textContent = testCount; + + $('next-test').disabled = testIndex == testCount - 1; + $('previous-test').disabled = testIndex == 0; + + $('test-link').href = + 'http://trac.webkit.org/browser/trunk/LayoutTests/' + testName; +} + +function getTestResultUrl(testName, mode) +{ + return '/test_result?test=' + testName + '&mode=' + mode; +} + +var currentExpectedImageTest; +var currentActualImageTest; + +function displayImageResults(testName) +{ + if (currentExpectedImageTest == currentActualImageTest + && currentExpectedImageTest == testName) { + return; + } + + function displayImageResult(mode, callback) { + var image = $(mode); + image.className = 'loading'; + image.src = getTestResultUrl(testName, mode); + image.onload = function() { + image.className = ''; + callback(); + updateImageDiff(); + }; + } + + displayImageResult( + 'expected-image', + function() { currentExpectedImageTest = testName; }); + displayImageResult( + 'actual-image', + function() { currentActualImageTest = testName; }); + + $('diff-canvas').className = 'loading'; + $('diff-canvas').style.display = ''; +} + +/** + * Computes a graphical a diff between the expected and actual images by + * rendering each to a canvas, getting the image data, and comparing the RGBA + * components of each pixel. The output is put into the diff canvas, with + * identical pixels appearing at 12.5% opacity and different pixels being + * highlighted in red. + */ +function updateImageDiff() { + if (currentExpectedImageTest != currentActualImageTest) + return; + + var expectedImage = $('expected-image'); + var actualImage = $('actual-image'); + + function getImageData(image) { + var imageCanvas = document.createElement('canvas'); + imageCanvas.width = image.width; + imageCanvas.height = image.height; + imageCanvasContext = imageCanvas.getContext('2d'); + + imageCanvasContext.fillStyle = 'rgba(255, 255, 255, 1)'; + imageCanvasContext.fillRect( + 0, 0, image.width, image.height); + + imageCanvasContext.drawImage(image, 0, 0); + return imageCanvasContext.getImageData( + 0, 0, image.width, image.height); + } + + var expectedImageData = getImageData(expectedImage); + var actualImageData = getImageData(actualImage); + + var diffCanvas = $('diff-canvas'); + var diffCanvasContext = diffCanvas.getContext('2d'); + var diffImageData = + diffCanvasContext.createImageData(diffCanvas.width, diffCanvas.height); + + // Avoiding property lookups for all these during the per-pixel loop below + // provides a significant performance benefit. + var expectedWidth = expectedImage.width; + var expectedHeight = expectedImage.height; + var expected = expectedImageData.data; + + var actualWidth = actualImage.width; + var actual = actualImageData.data; + + var diffWidth = diffImageData.width; + var diff = diffImageData.data; + + for (var x = 0; x < expectedWidth; x++) { + for (var y = 0; y < expectedHeight; y++) { + var expectedOffset = (y * expectedWidth + x) * 4; + var actualOffset = (y * actualWidth + x) * 4; + var diffOffset = (y * diffWidth + x) * 4; + if (expected[expectedOffset] != actual[actualOffset] || + expected[expectedOffset + 1] != actual[actualOffset + 1] || + expected[expectedOffset + 2] != actual[actualOffset + 2] || + expected[expectedOffset + 3] != actual[actualOffset + 3]) { + diff[diffOffset] = 255; + diff[diffOffset + 1] = 0; + diff[diffOffset + 2] = 0; + diff[diffOffset + 3] = 255; + } else { + diff[diffOffset] = expected[expectedOffset]; + diff[diffOffset + 1] = expected[expectedOffset + 1]; + diff[diffOffset + 2] = expected[expectedOffset + 2]; + diff[diffOffset + 3] = 32; + } + } + } + + diffCanvasContext.putImageData( + diffImageData, + 0, 0, + 0, 0, + diffImageData.width, diffImageData.height); + diffCanvas.className = ''; +} + +function displayTextResults(testName) +{ + function loadTextResult(mode) { + loadText(getTestResultUrl(testName, mode), function(text) { + $(mode).textContent = text; + }); + } + + loadTextResult('expected-text'); + loadTextResult('actual-text'); + loadTextResult('diff-text'); +} + +function nextTest() +{ + var testSelector = $('test-selector'); + var nextTestIndex = testSelector.selectedIndex + 1; + while (true) { + if (nextTestIndex == testSelector.options.length) { + return; + } + if (testSelector.options[nextTestIndex].disabled) { + nextTestIndex++; + } else { + testSelector.selectedIndex = nextTestIndex; + selectTest(); + return; + } + } +} + +function previousTest() +{ + var testSelector = $('test-selector'); + var previousTestIndex = testSelector.selectedIndex - 1; + while (true) { + if (previousTestIndex == -1) { + return; + } + if (testSelector.options[previousTestIndex].disabled) { + previousTestIndex--; + } else { + testSelector.selectedIndex = previousTestIndex; + selectTest(); + return + } + } +} + window.addEventListener('DOMContentLoaded', main); diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/util.js b/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/util.js new file mode 100644 index 0000000..1c8782b --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/util.js @@ -0,0 +1,57 @@ +/* + * Copyright (c) 2010 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. + */ + +var results; +var testsByFailureType = {}; +var testsByDirectory = {}; +var selectedTests = []; + +function $(id) +{ + return document.getElementById(id); +} + +function getSelectValue(id) +{ + var select = $(id); + if (select.selectedIndex == -1) { + return null; + } else { + return select.options[select.selectedIndex].value; + } +} + +function loadText(url, callback) +{ + var xhr = new XMLHttpRequest(); + xhr.open('GET', url); + xhr.addEventListener('load', function() { callback(xhr.responseText); }); + xhr.send(); +} diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/download.py b/WebKitTools/Scripts/webkitpy/tool/commands/download.py index 541c9c4..457c050 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/download.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/download.py @@ -29,8 +29,6 @@ import os -from optparse import make_option - import webkitpy.tool.steps as steps from webkitpy.common.checkout.changelog import ChangeLog, view_source_url diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py index bfca139..9ca343b 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py @@ -95,7 +95,7 @@ class DownloadCommandsTest(CommandsTest): self.assert_execute_outputs(ApplyFromBug(), [42], options=options, expected_stderr=expected_stderr) def test_land_diff(self): - expected_stderr = "Building WebKit\nRunning Python unit tests\nRunning Perl unit tests\nRunning JavaScriptCore tests\nRunning run-webkit-tests\nUpdating bug 42\n" + expected_stderr = "Building WebKit\nRunning Python unit tests\nRunning Perl unit tests\nRunning JavaScriptCore tests\nRunning run-webkit-tests\nCommitted r49824: <http://trac.webkit.org/changeset/49824>\nUpdating bug 42\n" mock_tool = MockTool() mock_tool.scm().create_patch = Mock() mock_tool.checkout().modified_changelogs = Mock(return_value=[]) @@ -105,7 +105,7 @@ class DownloadCommandsTest(CommandsTest): self.assertEqual(mock_tool.checkout().modified_changelogs.call_count, 1) def test_land_red_builders(self): - expected_stderr = '\nWARNING: Builders ["Builder2"] are red, please watch your commit carefully.\nSee http://dummy_buildbot_host/console?category=core\n\nBuilding WebKit\nRunning Python unit tests\nRunning Perl unit tests\nRunning JavaScriptCore tests\nRunning run-webkit-tests\nUpdating bug 42\n' + expected_stderr = '\nWARNING: Builders ["Builder2"] are red, please watch your commit carefully.\nSee http://dummy_buildbot_host/console?category=core\n\nBuilding WebKit\nRunning Python unit tests\nRunning Perl unit tests\nRunning JavaScriptCore tests\nRunning run-webkit-tests\nCommitted r49824: <http://trac.webkit.org/changeset/49824>\nUpdating bug 42\n' mock_tool = MockTool() mock_tool.buildbot.light_tree_on_fire() self.assert_execute_outputs(Land(), [42], options=self._default_options(), expected_stderr=expected_stderr, tool=mock_tool) @@ -128,6 +128,7 @@ Running Python unit tests Running Perl unit tests Running JavaScriptCore tests Running run-webkit-tests +Committed r49824: <http://trac.webkit.org/changeset/49824> Not closing bug 42 as attachment 197 has review=+. Assuming there are more patches to land from this bug. """ self.assert_execute_outputs(LandAttachment(), [197], options=self._default_options(), expected_stderr=expected_stderr) @@ -143,6 +144,7 @@ Running Python unit tests Running Perl unit tests Running JavaScriptCore tests Running run-webkit-tests +Committed r49824: <http://trac.webkit.org/changeset/49824> Not closing bug 42 as attachment 197 has review=+. Assuming there are more patches to land from this bug. Updating working directory Processing patch 128 from bug 42. @@ -151,6 +153,7 @@ Running Python unit tests Running Perl unit tests Running JavaScriptCore tests Running run-webkit-tests +Committed r49824: <http://trac.webkit.org/changeset/49824> Not closing bug 42 as attachment 197 has review=+. Assuming there are more patches to land from this bug. """ self.assert_execute_outputs(LandFromBug(), [42], options=self._default_options(), expected_stderr=expected_stderr) @@ -181,7 +184,7 @@ where ATTACHMENT_ID is the ID of this attachment. self.assert_execute_outputs(CreateRollout(), [852, "Reason"], options=self._default_options(), expected_stderr=expected_stderr) def test_rollout(self): - expected_stderr = "Preparing rollout for bug 42.\nUpdating working directory\nRunning prepare-ChangeLog\nMOCK: user.open_url: file://...\nBuilding WebKit\n" + expected_stderr = "Preparing rollout for bug 42.\nUpdating working directory\nRunning prepare-ChangeLog\nMOCK: user.open_url: file://...\nBuilding WebKit\nCommitted r49824: <http://trac.webkit.org/changeset/49824>\n" expected_stdout = "Was that diff correct?\n" self.assert_execute_outputs(Rollout(), [852, "Reason"], options=self._default_options(), expected_stdout=expected_stdout, expected_stderr=expected_stderr) diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py index 6b4213b..bfaeb08 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py @@ -38,15 +38,16 @@ from datetime import datetime from optparse import make_option from StringIO import StringIO -from webkitpy.common.net.bugzilla import CommitterValidator, Attachment +from webkitpy.common.config.committervalidator import CommitterValidator +from webkitpy.common.net.bugzilla import Attachment from webkitpy.common.net.layouttestresults import path_for_layout_test, LayoutTestResults from webkitpy.common.net.statusserver import StatusServer -from webkitpy.common.system.executive import ScriptError from webkitpy.common.system.deprecated_logging import error, log -from webkitpy.tool.commands.stepsequence import StepSequenceErrorHandler +from webkitpy.common.system.executive import ScriptError from webkitpy.tool.bot.commitqueuetask import CommitQueueTask, CommitQueueTaskDelegate from webkitpy.tool.bot.feeders import CommitQueueFeeder, EWSFeeder from webkitpy.tool.bot.queueengine import QueueEngine, QueueEngineDelegate +from webkitpy.tool.commands.stepsequence import StepSequenceErrorHandler from webkitpy.tool.grammar import pluralize, join_with_separators from webkitpy.tool.multicommandtool import Command, TryAgain diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/rebaselineserver.py b/WebKitTools/Scripts/webkitpy/tool/commands/rebaselineserver.py index 0a37677..abb2af4 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/rebaselineserver.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/rebaselineserver.py @@ -46,19 +46,22 @@ from optparse import make_option from wsgiref.handlers import format_date_time from webkitpy.tool.multicommandtool import AbstractDeclarativeCommand - +import webkitpy.thirdparty.simplejson as simplejson class RebaselineHTTPServer(BaseHTTPServer.HTTPServer): - def __init__(self, httpd_port, results_directory): + def __init__(self, httpd_port, results_directory, results_json): BaseHTTPServer.HTTPServer.__init__(self, ("", httpd_port), RebaselineHTTPRequestHandler) self.results_directory = results_directory + self.results_json = results_json class RebaselineHTTPRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): STATIC_FILE_NAMES = frozenset([ "index.html", + "loupe.js", "main.js", "main.css", + "util.js", ]) STATIC_FILE_DIRECTORY = os.path.join( @@ -111,6 +114,38 @@ class RebaselineHTTPRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): # otherwise there's a deadlock threading.Thread(target=lambda: self.server.shutdown()).start() + def test_result(self): + test_name, _ = os.path.splitext(self.query['test'][0]) + mode = self.query['mode'][0] + if mode == 'expected-image': + file_name = test_name + '-expected.png' + elif mode == 'actual-image': + file_name = test_name + '-actual.png' + if mode == 'expected-checksum': + file_name = test_name + '-expected.checksum' + elif mode == 'actual-checksum': + file_name = test_name + '-actual.checksum' + elif mode == 'diff-image': + file_name = test_name + '-diff.png' + if mode == 'expected-text': + file_name = test_name + '-expected.txt' + elif mode == 'actual-text': + file_name = test_name + '-actual.txt' + elif mode == 'diff-text': + file_name = test_name + '-diff.txt' + + file_path = os.path.join(self.server.results_directory, file_name) + + # Let results be cached for 60 seconds, so that they can be pre-fetched + # by the UI + self._serve_file(file_path, cacheable_seconds=60) + + def results_json(self): + self.send_response(200) + self.send_header('Content-type', 'application/json') + self.end_headers() + simplejson.dump(self.server.results_json, self.wfile) + def _serve_file(self, file_path, cacheable_seconds=0): if not os.path.exists(file_path): self.send_error(404, "File not found") @@ -147,11 +182,19 @@ class RebaselineServer(AbstractDeclarativeCommand): def execute(self, options, args, tool): results_directory = args[0] + print 'Parsing unexpected_results.json...' + results_json_path = os.path.join( + results_directory, 'unexpected_results.json') + with codecs.open(results_json_path, "r") as results_json_file: + results_json_file = file(results_json_path) + results_json = simplejson.load(results_json_file) + print "Starting server at http://localhost:%d/" % options.httpd_port print ("Use the 'Exit' link in the UI, http://localhost:%d/" "quitquitquit or Ctrl-C to stop") % options.httpd_port httpd = RebaselineHTTPServer( httpd_port=options.httpd_port, - results_directory=results_directory) + results_directory=results_directory, + results_json=results_json) httpd.serve_forever() diff --git a/WebKitTools/Scripts/webkitpy/tool/main.py b/WebKitTools/Scripts/webkitpy/tool/main.py index e0862c5..7b1d7f3 100755 --- a/WebKitTools/Scripts/webkitpy/tool/main.py +++ b/WebKitTools/Scripts/webkitpy/tool/main.py @@ -29,6 +29,7 @@ # # A tool for automating dealing with bugzilla, posting patches, committing patches, etc. +from optparse import make_option import os import threading @@ -38,20 +39,12 @@ from webkitpy.common.config.ports import WebKitPort from webkitpy.common.net.bugzilla import Bugzilla from webkitpy.common.net.buildbot import BuildBot from webkitpy.common.net.irc.ircproxy import IRCProxy +from webkitpy.common.net.statusserver import StatusServer from webkitpy.common.system.executive import Executive from webkitpy.common.system.user import User from webkitpy.layout_tests import port -import webkitpy.tool.commands as commands -# FIXME: Remove these imports once all the commands are in the root of the -# command package. -from webkitpy.tool.commands.download import * -from webkitpy.tool.commands.earlywarningsystem import * -from webkitpy.tool.commands.openbugs import OpenBugs -from webkitpy.tool.commands.queries import * -from webkitpy.tool.commands.queues import * -from webkitpy.tool.commands.sheriffbot import * -from webkitpy.tool.commands.upload import * from webkitpy.tool.multicommandtool import MultiCommandTool +import webkitpy.tool.commands as commands class WebKitPatch(MultiCommandTool): diff --git a/WebKitTools/Scripts/webkitpy/tool/mocktool.py b/WebKitTools/Scripts/webkitpy/tool/mocktool.py index b6ee95f..719f9b1 100644 --- a/WebKitTools/Scripts/webkitpy/tool/mocktool.py +++ b/WebKitTools/Scripts/webkitpy/tool/mocktool.py @@ -571,6 +571,7 @@ class MockStatusServer(object): # FIXME: This should not inherit from Mock +# FIXME: Unify with common.system.executive_mock.MockExecutive. class MockExecutive(Mock): def __init__(self, should_log): self._should_log = should_log diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/commit.py b/WebKitTools/Scripts/webkitpy/tool/steps/commit.py index 8f70b81..5c6bdb7 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/commit.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/commit.py @@ -26,7 +26,9 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +from webkitpy.common.checkout.changelog import view_source_url from webkitpy.common.checkout.scm import AuthenticationError, AmbiguousCommitError +from webkitpy.common.system.deprecated_logging import log from webkitpy.common.system.executive import ScriptError from webkitpy.common.system.user import User from webkitpy.tool.steps.abstractstep import AbstractStep @@ -61,7 +63,11 @@ class Commit(AbstractStep): num_tries += 1 try: - self._state["commit_text"] = self._tool.scm().commit_with_message(self._commit_message, git_commit=self._options.git_commit, username=username, force_squash=force_squash) + 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) + svn_revision = scm.svn_revision_from_commit_text(commit_text) + log("Committed r%s: <%s>" % (svn_revision, view_source_url(svn_revision))) + self._state["commit_text"] = commit_text break; except AmbiguousCommitError, e: if self._tool.user.confirm(self._commit_warning(e)): |