diff options
Diffstat (limited to 'WebKitTools/Scripts')
58 files changed, 1768 insertions, 511 deletions
diff --git a/WebKitTools/Scripts/VCSUtils.pm b/WebKitTools/Scripts/VCSUtils.pm index 24911ab..4516984 100644 --- a/WebKitTools/Scripts/VCSUtils.pm +++ b/WebKitTools/Scripts/VCSUtils.pm @@ -68,6 +68,7 @@ BEGIN { &pathRelativeToSVNRepositoryRootForPath &prepareParsedPatch &runPatchCommand + &scmToggleExecutableBit &setChangeLogDateAndReviewer &svnRevisionForDirectory &svnStatus @@ -90,6 +91,7 @@ my $changeLogTimeZone = "PST8PDT"; my $gitDiffStartRegEx = qr#^diff --git (\w/)?(.+) (\w/)?([^\r\n]+)#; my $svnDiffStartRegEx = qr#^Index: ([^\r\n]+)#; +my $svnPropertiesStartRegEx = qr#^Property changes on: ([^\r\n]+)#; # $1 is normally the same as the index path. my $svnPropertyStartRegEx = qr#^(Modified|Name|Added|Deleted): ([^\r\n]+)#; # $2 is the name of the property. my $svnPropertyValueStartRegEx = qr#^ (\+|-) ([^\r\n]+)#; # $2 is the start of the property's value (which may span multiple lines). @@ -735,6 +737,8 @@ sub parseDiffHeader($$) # # copiedFromPath: the path from which the file was copied if the diff # is a copy. +# executableBitDelta: the value 1 or -1 if the executable bit was added or +# removed from the target file, respectively. # indexPath: the path of the target file. For SVN-formatted diffs, # this is the same as the path in the "Index:" line. # isBinary: the value 1 if the diff is for a binary file. @@ -754,6 +758,13 @@ sub parseDiffHeader($$) # # This subroutine preserves any leading junk encountered before the header. # +# Composition of an SVN diff +# +# There are three parts to an SVN diff: the header, the property change, and +# the binary contents, in that order. Either the header or the property change +# may be ommitted, but not both. If there are binary changes, then you always +# have all three. +# # Args: # $fileHandle: a file handle advanced to the first line of the next # header block. Leading junk is okay. @@ -773,6 +784,7 @@ sub parseDiff($$) my $headerStartRegEx = $svnDiffStartRegEx; # SVN-style header for the default my $headerHashRef; # Last header found, as returned by parseDiffHeader(). + my $svnPropertiesHashRef; # Last SVN properties diff found, as returned by parseSvnDiffProperties(). my $svnText; while (defined($line)) { if (!$headerHashRef && ($line =~ $gitDiffStartRegEx)) { @@ -782,6 +794,18 @@ sub parseDiff($$) $headerStartRegEx = $gitDiffStartRegEx; } + if ($line =~ $svnPropertiesStartRegEx) { + my $propertyPath = $1; + if ($svnPropertiesHashRef || $headerHashRef && ($propertyPath ne $headerHashRef->{indexPath})) { + # This is the start of the second diff in the while loop, which happens to + # be a property diff. If $svnPropertiesHasRef is defined, then this is the + # second consecutive property diff, otherwise it's the start of a property + # diff for a file that only has property changes. + last; + } + ($svnPropertiesHashRef, $line) = parseSvnDiffProperties($fileHandle, $line); + next; + } if ($line !~ $headerStartRegEx) { # Then we are in the body of the diff. $svnText .= $line; @@ -789,8 +813,9 @@ sub parseDiff($$) next; } # Otherwise, we found a diff header. - if ($headerHashRef) { - # Then this is the second diff header of this while loop. + if ($svnPropertiesHashRef || $headerHashRef) { + # Then either we just processed an SVN property change or this + # is the start of the second diff header of this while loop. last; } @@ -812,12 +837,29 @@ sub parseDiff($$) $copyHash{copiedFromPath} = $headerHashRef->{copiedFromPath}; $copyHash{indexPath} = $headerHashRef->{indexPath}; $copyHash{sourceRevision} = $headerHashRef->{sourceRevision} if $headerHashRef->{sourceRevision}; + if ($headerHashRef->{isSvn}) { + $copyHash{executableBitDelta} = $svnPropertiesHashRef->{executableBitDelta} if $svnPropertiesHashRef->{executableBitDelta}; + } push @diffHashRefs, \%copyHash; } - if (!$headerHashRef->{copiedFromPath} || $headerHashRef->{isCopyWithChanges}) { + + # Note, the order of evaluation for the following if conditional has been explicitly chosen so that + # it evaluates to false when there is no headerHashRef (e.g. a property change diff for a file that + # only has property changes). + if ($headerHashRef->{isCopyWithChanges} || (%$headerHashRef && !$headerHashRef->{copiedFromPath})) { # Then add the usual file modification. my %diffHash; - # FIXME: Add executableBitDelta as a key. + # FIXME: We should expand this code to support other properties. In the future, + # parseSvnDiffProperties may return a hash whose keys are the properties. + if ($headerHashRef->{isSvn}) { + # SVN records the change to the executable bit in a separate property change diff + # that follows the contents of the diff, except for binary diffs. For binary + # diffs, the property change diff follows the diff header. + $diffHash{executableBitDelta} = $svnPropertiesHashRef->{executableBitDelta} if $svnPropertiesHashRef->{executableBitDelta}; + } elsif ($headerHashRef->{isGit}) { + # Git records the change to the executable bit in the header of a diff. + $diffHash{executableBitDelta} = $headerHashRef->{executableBitDelta} if $headerHashRef->{executableBitDelta}; + } $diffHash{indexPath} = $headerHashRef->{indexPath}; $diffHash{isBinary} = $headerHashRef->{isBinary} if $headerHashRef->{isBinary}; $diffHash{isDeletion} = $headerHashRef->{isDeletion} if $headerHashRef->{isDeletion}; @@ -831,10 +873,24 @@ sub parseDiff($$) } # FIXME: Remove the need for svnConvertedText. See the %diffHash # code comments above for more information. - $diffHash{svnConvertedText} = $svnText; + # + # Note, we may not always have SVN converted text since we intend + # to deprecate it in the future. For example, a property change + # diff for a file that only has property changes will not return + # any SVN converted text. + $diffHash{svnConvertedText} = $svnText if $svnText; push @diffHashRefs, \%diffHash; } + if (!%$headerHashRef && $svnPropertiesHashRef) { + # A property change diff for a file that only has property changes. + my %propertyChangeHash; + $propertyChangeHash{executableBitDelta} = $svnPropertiesHashRef->{executableBitDelta} if $svnPropertiesHashRef->{executableBitDelta}; + $propertyChangeHash{indexPath} = $svnPropertiesHashRef->{propertyPath}; + $propertyChangeHash{isSvn} = 1; + push @diffHashRefs, \%propertyChangeHash; + } + return (\@diffHashRefs, $line); } @@ -859,19 +915,14 @@ sub parseDiff($$) # executableBitDelta: the value 1 or -1 if the executable bit was added or # removed from the target file, respectively. # $lastReadLine: the line last read from $fileHandle. -# -# FIXME: This method is unused as of (05/22/2010). We will call this function -# as part of parsing a diff. See <https://bugs.webkit.org/show_bug.cgi?id=39409>. sub parseSvnDiffProperties($$) { my ($fileHandle, $line) = @_; $_ = $line; - my $svnFooterDiffStartRegEx = qr#Property changes on: ([^\r\n]+)#; # $1 is normally the same as the index path. - my %footer; - if (/$svnFooterDiffStartRegEx/) { + if (/$svnPropertiesStartRegEx/) { $footer{propertyPath} = $1; } else { die("Failed to find start of SVN property change, \"Property changes on \": \"$_\""); @@ -1300,7 +1351,7 @@ sub fixChangeLogPatch($) # Work backwards, shifting overlapping lines towards front # while checking that patch stays equivalent. - for ($i = $dateStartIndex - 1; $i >= $chunkStartIndex; --$i) { + for ($i = $dateStartIndex - 1; @overlappingLines && $i >= $chunkStartIndex; --$i) { my $line = $lines[$i]; if (substr($line, 0, 1) ne " ") { next; diff --git a/WebKitTools/Scripts/build-webkit b/WebKitTools/Scripts/build-webkit index 2934b21..18a557b 100755 --- a/WebKitTools/Scripts/build-webkit +++ b/WebKitTools/Scripts/build-webkit @@ -55,7 +55,7 @@ my $prefixPath; my $makeArgs; my $startTime = time(); -my ($threeDCanvasSupport, $threeDRenderingSupport, $channelMessagingSupport, $clientBasedGeolocationSupport, $databaseSupport, $datagridSupport, $datalistSupport, +my ($linkPrefetchSupport, $threeDCanvasSupport, $threeDRenderingSupport, $channelMessagingSupport, $clientBasedGeolocationSupport, $databaseSupport, $datagridSupport, $datalistSupport, $domStorageSupport, $eventsourceSupport, $filtersSupport, $geolocationSupport, $iconDatabaseSupport, $imageResizerSupport, $indexedDatabaseSupport, $inputSpeechSupport, $javaScriptDebuggerSupport, $mathmlSupport, $offlineWebApplicationSupport, $rubySupport, $systemMallocSupport, $sandboxSupport, $sharedWorkersSupport, $svgSupport, $svgAnimationSupport, $svgAsImageSupport, $svgDOMObjCBindingsSupport, $svgFontsSupport, @@ -64,6 +64,9 @@ my ($threeDCanvasSupport, $threeDRenderingSupport, $channelMessagingSupport, $cl $fileReaderSupport, $fileWriterSupport, $directoryUploadSupport); my @features = ( + { option => "link-prefetch", desc => "Toggle pre fetching support", + define => "ENABLE_LINK_PREFETCH", default => 0, value => \$linkPrefetchSupport }, + { option => "3d-canvas", desc => "Toggle 3D canvas support", define => "ENABLE_3D_CANVAS", default => (isAppleMacWebKit() && !isTiger()), value => \$threeDCanvasSupport }, diff --git a/WebKitTools/Scripts/check-header-includes b/WebKitTools/Scripts/check-header-includes new file mode 100755 index 0000000..568859e --- /dev/null +++ b/WebKitTools/Scripts/check-header-includes @@ -0,0 +1,103 @@ +#!/usr/bin/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. +# +# This script looks for unnecessary #includes in the given header file, +# or in all header files that can be recursively found in the given directory. + +import os +import re +import sys + +headerFile = re.compile(r'\.h$') +includeLine = re.compile(r'^#include ("|<wtf/)([A-Z]\w+).h[>"]') +smartPointerString = "(ListRefPtr|OwnArrayPtr|OwnPtr|PassOwnPtr|PassRefPtr|RefPtr)<className>" +allowedIncludes = set(["Forward", "PlatformString", "StringHash", "Threading"]) + +# Check if 'line' needs any class in 'unnecessaryIncludes' to be included in the header file. +def checkLine(line, unnecessaryIncludes): + unnecessaryIncludesCopy = unnecessaryIncludes.copy() + for includedClass in unnecessaryIncludesCopy: + if re.search(includedClass, line): + if re.search(includedClass + "\*", line): + continue + + smartPointerClassString = smartPointerString.replace("className", includedClass) + if re.search(smartPointerClassString, line): + continue + + unnecessaryIncludes.remove(includedClass) + + +# Look for unnecessary includes in the given file +def checkFile(fileName): + if headerFile.search(fileName): + f = open(fileName, "r") + unnecessaryIncludes = set() + for line in f: + match = includeLine.match(line) + if match: + className = match.group(2) + if className == "PlatformString": + unnecessaryIncludes.add("String") + elif className not in allowedIncludes: + unnecessaryIncludes.add(className) + else: + checkLine(line, unnecessaryIncludes) + + if len(unnecessaryIncludes) > 0: + print fileName + for includedClass in unnecessaryIncludes: + print includedClass + print + + +# If 'path' is a file, check if it has unnecessary includes. +# If 'path' is a directory, traverse it recursively and look +# for unnecessary includes in all header files. +def checkPath(path): + if os.path.isfile(path): + checkFile(path) + elif os.path.isdir(path): + dirList = os.listdir(path) + for dirName in dirList: + checkPath(path + "/" + dirName) + else: + print path, "is not a file or directory." + + +def main(): + if len(sys.argv) <= 1: + print "Usage:", sys.argv[0], "<path>" + return -1 + + checkPath(sys.argv[1]) + + +if __name__ == "__main__": + main() diff --git a/WebKitTools/Scripts/check-webkit-style b/WebKitTools/Scripts/check-webkit-style index 19d3762..9a1ae1e 100755 --- a/WebKitTools/Scripts/check-webkit-style +++ b/WebKitTools/Scripts/check-webkit-style @@ -115,7 +115,7 @@ def main(): if paths: file_reader.process_paths(paths) else: - patch = checkout.create_patch(options.git_commit, options.squash) + patch = checkout.create_patch(options.git_commit) patch_checker = PatchReader(file_reader) patch_checker.check(patch) diff --git a/WebKitTools/Scripts/ensure-valid-python b/WebKitTools/Scripts/ensure-valid-python index 4640a01..aede812 100755 --- a/WebKitTools/Scripts/ensure-valid-python +++ b/WebKitTools/Scripts/ensure-valid-python @@ -42,7 +42,7 @@ my $macPythonMD5 = "84489bba813fdbb6041b69d4310a86da"; my $macPythonInstallerName = "Python.mpkg"; # We could use a consistent download location, like the source or build directory. -my $tempDirectory = File::Temp->tempdir("WebKitPythonXXXX"); +my $tempDirectory = File::Temp::tempdir("WebKitPythonXXXX", TMPDIR => 1, CLEANUP => 1); my $downloadDirectory = $tempDirectory; my $mountPoint = File::Spec->join($tempDirectory, "mount"); diff --git a/WebKitTools/Scripts/old-run-webkit-tests b/WebKitTools/Scripts/old-run-webkit-tests index 0e705a9..30c585f 100755 --- a/WebKitTools/Scripts/old-run-webkit-tests +++ b/WebKitTools/Scripts/old-run-webkit-tests @@ -120,6 +120,7 @@ sub writeToFile($$); my $addPlatformExceptions = 0; my $complexText = 0; my $exitAfterNFailures = 0; +my $exitAfterNCrashesOrTimeouts = 0; my $generateNewResults = isAppleMacWebKit() ? 1 : 0; my $guardMalloc = ''; # FIXME: Dynamic HTTP-port configuration in this file is wrong. The various @@ -156,7 +157,7 @@ my $tmpDir = "/tmp"; my $testResultsDirectory = File::Spec->catfile($tmpDir, "layout-test-results"); my $testsPerDumpTool = 1000; my $threaded = 0; -my $html5parser = 0; +my $html5treebuilder = 0; # DumpRenderTree has an internal timeout of 30 seconds, so this must be > 30. my $timeoutSeconds = 35; my $tolerance = 0; @@ -183,7 +184,10 @@ my $prettyDiffTag = "pretty-diff"; my $diffsTag = "diffs"; my $errorTag = "stderr"; +my $realPlatform; + my @macPlatforms = ("mac-tiger", "mac-leopard", "mac-snowleopard", "mac"); +my @winPlatforms = ("win-xp", "win-vista", "win-7", "win"); if (isAppleMacWebKit()) { if (isTiger()) { @@ -219,7 +223,15 @@ if (isAppleMacWebKit()) { } elsif (isWx()) { $platform = "wx"; } elsif (isCygwin()) { - $platform = "win"; + if (isWindowsXP()) { + $platform = "win-xp"; + } elsif (isWindowsVista()) { + $platform = "win-vista"; + } elsif (isWindows7()) { + $platform = "win-7"; + } else { + $platform = "win"; + } } if (!defined($platform)) { @@ -243,7 +255,9 @@ Usage: $programName [options] [testdir|testpath ...] --complex-text Use the complex text code path for all text (Mac OS X and Windows only) -c|--configuration config Set DumpRenderTree build configuration -g|--guard-malloc Enable malloc guard - --exit-after-n-failures N Exit after the first N failures instead of running all tests + --exit-after-n-failures N Exit after the first N failures (includes crashes) instead of running all tests + --exit-after-n-crashes-or-timeouts N + Exit after the first N crashes instead of running all tests -h|--help Show this help message --[no-]http Run (or do not run) http tests (default: $httpDefault) --[no-]wait-for-httpd Wait for httpd if some other test session is using it already (same as WEBKIT_WAIT_FOR_HTTPD=1). (default: $shouldWaitForHTTPD) @@ -274,7 +288,7 @@ Usage: $programName [options] [testdir|testpath ...] --ignore-metrics Ignore metrics in tests --[no-]strip-editing-callbacks Remove editing callbacks from expected results -t|--threaded Run a concurrent JavaScript thead with each test - --html5-parser Run the tests using the HTML5 parser + --html5-treebuilder Run the tests using the HTML5 tree builder --timeout t Sets the number of seconds before a test times out (default: $timeoutSeconds) --valgrind Run DumpRenderTree inside valgrind (Qt/Linux only) -v|--verbose More verbose output (overrides --quiet) @@ -289,6 +303,7 @@ my $getOptionsResult = GetOptions( 'add-platform-exceptions' => \$addPlatformExceptions, 'complex-text' => \$complexText, 'exit-after-n-failures=i' => \$exitAfterNFailures, + 'exit-after-n-crashes-or-timeouts=i' => \$exitAfterNCrashesOrTimeouts, 'guard-malloc|g' => \$guardMalloc, 'help|h' => \$showHelp, 'http!' => \$testHTTP, @@ -317,7 +332,7 @@ my $getOptionsResult = GetOptions( 'slowest' => \$report10Slowest, 'strip-editing-callbacks!' => \$stripEditingCallbacks, 'threaded|t' => \$threaded, - 'html5-parser' => \$html5parser, + 'html5-treebuilder' => \$html5treebuilder, 'timeout=i' => \$timeoutSeconds, 'tolerance=f' => \$tolerance, 'use-remote-links-to-tests' => \$useRemoteLinksToTests, @@ -331,6 +346,14 @@ if (!$getOptionsResult || $showHelp) { exit 1; } +if ($useWebKitTestRunner) { + if (isAppleMacWebKit()) { + $realPlatform = $platform; + $platform = "mac-wk2"; + } +} + + my $ignoreSkipped = $treatSkipped eq "ignore"; my $skippedOnly = $treatSkipped eq "only"; @@ -556,7 +579,7 @@ my $totalLeaks = 0; my @toolArgs = (); push @toolArgs, "--pixel-tests" if $pixelTests; push @toolArgs, "--threaded" if $threaded; -push @toolArgs, "--html5-parser" if $html5parser; +push @toolArgs, "--html5-treebuilder" if $html5treebuilder; push @toolArgs, "--complex-text" if $complexText; push @toolArgs, "-"; @@ -1370,7 +1393,11 @@ sub openDumpTool() setPathForRunningWebKitApp(\%CLEAN_ENV); } - # Port spesifics + # Port specifics + if (isGtk()) { + $CLEAN_ENV{GTK_MODULES} = "gail"; + } + if (isQt()) { $CLEAN_ENV{QTWEBKIT_PLUGIN_PATH} = productDir() . "/lib/plugins"; $CLEAN_ENV{QT_DRT_WEBVIEW_MODE} = $ENV{"QT_DRT_WEBVIEW_MODE"}; @@ -1852,13 +1879,23 @@ sub buildPlatformResultHierarchy() mkpath($platformTestDirectory) if ($platform eq "undefined" && !-d "$platformTestDirectory"); my @platforms; - if ($platform =~ /^mac-/) { + + my $isMac = $platform =~ /^mac/; + my $isWin = $platform =~ /^win/; + if ($isMac || $isWin) { + my $effectivePlatform = $platform; + if ($platform eq "mac-wk2") { + push @platforms, "mac-wk2"; + $effectivePlatform = $realPlatform; + } + + my @platformList = $isMac ? @macPlatforms : @winPlatforms; my $i; - for ($i = 0; $i < @macPlatforms; $i++) { - last if $macPlatforms[$i] eq $platform; + for ($i = 0; $i < @platformList; $i++) { + last if $platformList[$i] eq $effectivePlatform; } - for (; $i < @macPlatforms; $i++) { - push @platforms, $macPlatforms[$i]; + for (; $i < @platformList; $i++) { + push @platforms, $platformList[$i]; } } elsif ($platform =~ /^qt-/) { push @platforms, $platform; @@ -1872,7 +1909,7 @@ sub buildPlatformResultHierarchy() my $scoped = catdir($platformBaseDirectory, $platforms[$i]); push(@hierarchy, $scoped) if (-d $scoped); } - + return @hierarchy; } @@ -1880,7 +1917,9 @@ sub buildPlatformTestHierarchy(@) { my (@platformHierarchy) = @_; return @platformHierarchy if (@platformHierarchy < 2); - + if ($platformHierarchy[0] =~ /mac-wk2/) { + return ($platformHierarchy[0], $platformHierarchy[1], $platformHierarchy[$#platformHierarchy]); + } return ($platformHierarchy[0], $platformHierarchy[$#platformHierarchy]); } @@ -2264,14 +2303,24 @@ sub printResults sub stopRunningTestsEarlyIfNeeded() { # --reset-results does not check pass vs. fail, so exitAfterNFailures makes no sense with --reset-results. - return 0 if !$exitAfterNFailures || $resetResults; + return 0 if $resetResults; my $passCount = $counts{match} || 0; # $counts{match} will be undefined if we've not yet passed a test (e.g. the first test fails). my $newCount = $counts{new} || 0; my $failureCount = $count - $passCount - $newCount; # "Failure" here includes timeouts, crashes, etc. - return 0 if $failureCount < $exitAfterNFailures; + if ($exitAfterNFailures && $failureCount >= $exitAfterNFailures) { + print "\nExiting early after $failureCount failures. $count tests run."; + closeDumpTool(); + return 1; + } - print "\nExiting early after $failureCount failures. $count tests run."; - closeDumpTool(); - return 1; + my $crashCount = $counts{crash} || 0; + my $timeoutCount = $counts{timedout} || 0; + if ($exitAfterNCrashesOrTimeouts && $crashCount + $timeoutCount >= $exitAfterNCrashesOrTimeouts) { + print "\nExiting early after $crashCount crashes and $timeoutCount timeouts. $count tests run."; + closeDumpTool(); + return 1; + } + + return 0; } diff --git a/WebKitTools/Scripts/run-qtwebkit-tests b/WebKitTools/Scripts/run-qtwebkit-tests new file mode 100644 index 0000000..373de0a --- /dev/null +++ b/WebKitTools/Scripts/run-qtwebkit-tests @@ -0,0 +1,358 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +#Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies) + +#This library is free software; you can redistribute it and/or +#modify it under the terms of the GNU Library General Public +#License as published by the Free Software Foundation; either +#version 2 of the License, or (at your option) any later version. + +#This library is distributed in the hope that it will be useful, +#but WITHOUT ANY WARRANTY; without even the implied warranty of +#MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +#Library General Public License for more details. + +#You should have received a copy of the GNU Library General Public License +#along with this library; see the file COPYING.LIB. If not, write to +#the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, +#Boston, MA 02110-1301, USA. + +from __future__ import with_statement + +import sys +import os +import os.path +import re +import logging +from subprocess import Popen, PIPE, STDOUT +from optparse import OptionParser + + +class Log(object): + def __init__(self, name): + self._log = logging.getLogger(name) + self.debug = self._log.debug + self.warn = self._log.warn + self.error = self._log.error + self.exception = self._log.exception + self.info = self._log.info + + +class Options(Log): + """ Option manager. It parses and checks script's parameters, sets an internal variable. """ + + def __init__(self, args): + Log.__init__(self, "Options") + log = self._log + opt = OptionParser("%prog [options] PathToSearch.\nTry -h or --help.") + opt.add_option("-j", "--parallel-level", action="store", type="int", + dest="parallel_level", default=None, + help="Number of parallel processes executing the Qt's tests. Default: cpu count.") + opt.add_option("-v", "--verbose", action="store", type="int", + dest="verbose", default=2, + help="Verbose level (0 - quiet, 1 - errors only, 2 - infos and warnings, 3 - debug information). Default: %default.") + opt.add_option("", "--tests-options", action="store", type="string", + dest="tests_options", default="", + help="Parameters passed to Qt's tests (for example '-eventdelay 123').") + opt.add_option("-o", "--output-file", action="store", type="string", + dest="output_file", default="/tmp/qtwebkittests.html", + help="File where results will be stored. The file will be overwritten. Default: %default.") + opt.add_option("-b", "--browser", action="store", dest="browser", + default="xdg-open", + help="Browser in which results will be opened. Default %default.") + opt.add_option("", "--do-not-open-results", action="store_false", + dest="open_results", default=True, + help="The results shouldn't pop-up in a browser automatically") + opt.add_option("-d", "--developer-mode", action="store_true", + dest="developer", default=False, + help="Special mode for debugging. In general it simulates human behavior, running all autotests. In the mode everything is executed synchronously, no html output will be generated, no changes or transformation will be applied to stderr or stdout. In this mode options; parallel-level, output-file, browser and do-not-open-results will be ignored.") + + self._o, self._a = opt.parse_args(args) + verbose = self._o.verbose + if verbose == 0: + logging.basicConfig(level=logging.CRITICAL,) + elif verbose == 1: + logging.basicConfig(level=logging.ERROR,) + elif verbose == 2: + logging.basicConfig(level=logging.INFO,) + elif verbose == 3: + logging.basicConfig(level=logging.DEBUG,) + else: + logging.basicConfig(level=logging.INFO,) + log.warn("Bad verbose level, switching to default.") + try: + if not os.path.exists(self._a[0]): + raise Exception("Given path doesn't exist.") + if len(self._a) > 1: + raise IndexError("Only one directory could be provided.") + self._o.path = self._a[0] + except IndexError: + log.error("Bad usage. Please try -h or --help.") + sys.exit(1) + except Exception: + log.error("Path '%s' doesn't exist", self._a[0]) + sys.exit(2) + if self._o.developer: + if not self._o.parallel_level is None: + log.warn("Developer mode sets parallel-level option to one.") + self._o.parallel_level = 1 + self._o.open_results = False + + def __getattr__(self, attr): + """ Maps all options properties into this object (remove one level of indirection). """ + return getattr(self._o, attr) + + +def run_test(args): + """ Runs one given test. + args should contain a tuple with 3 elements; + TestSuiteResult containing full file name of an autotest executable. + str with options that should be passed to the autotest executable + bool if true then the stdout will be buffered and separated from the stderr, if it is false + then the stdout and the stderr will be merged together and left unbuffered (the TestSuiteResult output will be None). + """ + log = logging.getLogger("Exec") + test_suite, options, buffered = args + try: + log.info("Running... %s", test_suite.test_file_name()) + if buffered: + tst = Popen(test_suite.test_file_name() + options, stdout=PIPE, stderr=None, shell=True) + else: + tst = Popen(test_suite.test_file_name() + options, stdout=None, stderr=STDOUT, shell=True) + except OSError, e: + log.exception("Can't open an autotest file: '%s'. Skipping the test...", e.filename) + else: + test_suite.set_output(tst.communicate()[0]) # takes stdout only, in developer mode it would be None. + log.info("Finished %s", test_suite.test_file_name()) + return test_suite + + +class TestSuiteResult(object): + """ Keeps information about a test. """ + + def __init__(self): + self._output = None + self._test_file_name = None + + def set_output(self, xml): + if xml: + self._output = xml.strip() + + def output(self): + return self._output + + def set_test_file_name(self, file_name): + self._test_file_name = file_name + + def test_file_name(self): + return self._test_file_name + + +class Main(Log): + """ The main script. All real work is done in run() method. """ + + def __init__(self, options): + Log.__init__(self, "Main") + self._options = options + if options.parallel_level > 1 or options.parallel_level is None: + try: + from multiprocessing import Pool + except ImportError: + self.warn("Import Error: the multiprocessing module couldn't be loaded (may be lack of python-multiprocessing package?). The Qt autotests will be executed one by one.") + options.parallel_level = 1 + if options.parallel_level == 1: + + class Pool(object): + """ A hack, created to avoid problems with multiprocessing module, this class is single thread replacement for the multiprocessing.Pool class. """ + def __init__(self, processes): + pass + + def imap_unordered(self, func, files): + return map(func, files) + + def map(self, func, files): + return map(func, files) + + self._Pool = Pool + + def run(self): + """ Find && execute && publish results of all test. "All in one" function. """ + self.debug("Searching executables...") + tests_executables = self.find_tests_paths(self._options.path) + self.debug("Found: %s", len(tests_executables)) + self.debug("Executing tests...") + results = self.run_tests(tests_executables) + if not self._options.developer: + self.debug("Transforming...") + transformed_results = self.transform(results) + self.debug("Publishing...") + self.announce_results(transformed_results) + + def find_tests_paths(self, path): + """ Finds all tests executables inside the given path. """ + executables = [] + for root, dirs, files in os.walk(path): + # Check only for a file that name starts from 'tst_' and that we can execute. + filtered_path = filter(lambda w: w.startswith('tst_') and os.access(os.path.join(root, w), os.X_OK), files) + filtered_path = map(lambda w: os.path.join(root, w), filtered_path) + for file_name in filtered_path: + r = TestSuiteResult() + r.set_test_file_name(file_name) + executables.append(r) + return executables + + def run_tests(self, files): + """ Executes given files by using a pool of workers. """ + workers = self._Pool(processes=self._options.parallel_level) + # to each file add options. + self.debug("Using %s the workers pool, number of workers %i", repr(workers), self._options.parallel_level) + package = map(lambda w: [w, self._options.tests_options, not self._options.developer], files) + self.debug("Generated packages for workers: %s", repr(package)) + results = workers.map(run_test, package) # Collects results. + return results + + def transform(self, results): + """ Transforms list of the results to specialized versions. """ + stdout = self.convert_to_stdout(results) + html = self.convert_to_html(results) + return {"stdout": stdout, "html": html} + + def announce_results(self, results): + """ Shows the results. """ + self.announce_results_stdout(results['stdout']) + self.announce_results_html(results['html']) + + def announce_results_stdout(self, results): + """ Show the results by printing to the stdout.""" + print(results) + + def announce_results_html(self, results): + """ Shows the result by creating a html file and calling a web browser to render it. """ + with file(self._options.output_file, 'w') as f: + f.write(results) + if self._options.open_results: + Popen(self._options.browser + " " + self._options.output_file, stdout=None, stderr=None, shell=True) + + def convert_to_stdout(self, results): + """ Converts results, that they could be nicely presented in the stdout. """ + # Join all results into one piece. + txt = "\n\n".join(map(lambda w: w.output(), results)) + # Find total count of failed, skipped and passed tests. + totals = re.findall(r"([0-9]+) passed, ([0-9]+) failed, ([0-9]+) skipped", txt) + totals = reduce(lambda x, y: (int(x[0]) + int(y[0]), int(x[1]) + int(y[1]), int(x[2]) + int(y[2])), totals) + totals = map(str, totals) + totals = totals[0] + " passed, " + totals[1] + " failed, " + totals[2] + " skipped" + # Add a summary. + txt += '\n\n\n' + '*' * 70 + txt += "\n**" + ("TOTALS: " + totals).center(66) + '**' + txt += '\n' + '*' * 70 + '\n' + return txt + + def convert_to_html(self, results): + """ Converts results, that they could showed as a html page. """ + # Join results into one piece. + txt = "\n\n".join(map(lambda w: w.output(), results)) + txt = txt.replace('&', '&').replace('<', "<").replace('>', ">") + # Add a color and a style. + txt = re.sub(r"([* ]+(Finished)[ a-z_A-Z0-9]+[*]+)", + lambda w: r"", + txt) + txt = re.sub(r"([*]+[ a-z_A-Z0-9]+[*]+)", + lambda w: "<case class='good'><br><br><b>" + w.group(0) + r"</b></case>", + txt) + txt = re.sub(r"(Config: Using QTest library)((.)+)", + lambda w: "\n<case class='good'><br><i>" + w.group(0) + r"</i> ", + txt) + txt = re.sub(r"\n(PASS)((.)+)", + lambda w: "</case>\n<case class='good'><br><status class='pass'>" + w.group(1) + r"</status>" + w.group(2), + txt) + txt = re.sub(r"\n(FAIL!)((.)+)", + lambda w: "</case>\n<case class='bad'><br><status class='fail'>" + w.group(1) + r"</status>" + w.group(2), + txt) + txt = re.sub(r"\n(XPASS)((.)+)", + lambda w: "</case>\n<case class='bad'><br><status class='xpass'>" + w.group(1) + r"</status>" + w.group(2), + txt) + txt = re.sub(r"\n(XFAIL)((.)+)", + lambda w: "</case>\n<case class='good'><br><status class='xfail'>" + w.group(1) + r"</status>" + w.group(2), + txt) + txt = re.sub(r"\n(SKIP)((.)+)", + lambda w: "</case>\n<case class='good'><br><status class='xfail'>" + w.group(1) + r"</status>" + w.group(2), + txt) + txt = re.sub(r"\n(QWARN)((.)+)", + lambda w: "</case>\n<case class='bad'><br><status class='warn'>" + w.group(1) + r"</status>" + w.group(2), + txt) + txt = re.sub(r"\n(RESULT)((.)+)", + lambda w: "</case>\n<case class='good'><br><status class='benchmark'>" + w.group(1) + r"</status>" + w.group(2), + txt) + txt = re.sub(r"\n(QFATAL)((.)+)", + lambda w: "</case>\n<case class='bad'><br><status class='crash'>" + w.group(1) + r"</status>" + w.group(2), + txt) + txt = re.sub(r"\n(Totals:)([0-9', a-z]*)", + lambda w: "</case>\n<case class='good'><br><b>" + w.group(1) + r"</b>" + w.group(2) + "</case>", + txt) + # Find total count of failed, skipped and passed tests. + totals = re.findall(r"([0-9]+) passed, ([0-9]+) failed, ([0-9]+) skipped", txt) + totals = reduce(lambda x, y: (int(x[0]) + int(y[0]), int(x[1]) + int(y[1]), int(x[2]) + int(y[2])), totals) + totals = map(str, totals) + totals = totals[0] + " passed, " + totals[1] + " failed, " + totals[2] + " skipped." + # Create a header of the html source. + txt = """ + <html> + <head> + <script> + function init() { + // Try to find the right styleSheet (this document could be embedded in an other html doc) + for (i = document.styleSheets.length - 1; i >= 0; --i) { + if (document.styleSheets[i].cssRules[0].selectorText == "case.good") { + resultStyleSheet = i; + return; + } + } + // The styleSheet hasn't been found, but it should be the last one. + resultStyleSheet = document.styleSheets.length - 1; + } + + function hide() { + document.styleSheets[resultStyleSheet].cssRules[0].style.display='none'; + } + + function show() { + document.styleSheets[resultStyleSheet].cssRules[0].style.display=''; + } + + </script> + <style type="text/css"> + case.good {color:black} + case.bad {color:black} + status.pass {color:green} + status.crash {color:red} + status.fail {color:red} + status.xpass {color:663300} + status.xfail {color:004500} + status.benchmark {color:000088} + status.warn {color:orange} + status.crash {color:red; text-decoration:blink; background-color:black} + </style> + </head> + <body onload="init()"> + <center> + <h1>Qt's autotests results</h1>%(totals)s<br> + <hr> + <form> + <input type="button" value="Show failures only" onclick="hide()"/> + + <input type="button" value="Show all" onclick="show()"/> + </form> + </center> + <hr> + %(results)s + </body> + </html>""" % {"totals": totals, "results": txt} + return txt + + +if __name__ == '__main__': + options = Options(sys.argv[1:]) + main = Main(options) + main.run() diff --git a/WebKitTools/Scripts/webkitdirs.pm b/WebKitTools/Scripts/webkitdirs.pm index 9f54c3e..674dc8c 100644 --- a/WebKitTools/Scripts/webkitdirs.pm +++ b/WebKitTools/Scripts/webkitdirs.pm @@ -73,6 +73,7 @@ my $isInspectorFrontend; my $vcBuildPath; my $windowsTmpPath; my $windowsSourceDir; +my $winVersion; my $willUseVCExpressWhenBuilding = 0; # Defined in VCSUtils. @@ -799,6 +800,41 @@ sub isCygwin() return ($^O eq "cygwin") || 0; } +sub determineWinVersion() +{ + return if $winVersion; + + if (!isCygwin()) { + $winVersion = -1; + return; + } + + my $versionString = `uname -s`; + $versionString =~ /(\d\.\d)/; + $winVersion = $1; +} + +sub winVersion() +{ + determineWinVersion(); + return $winVersion; +} + +sub isWindows7() +{ + return winVersion() eq "6.1"; +} + +sub isWindowsVista() +{ + return winVersion() eq "6.0"; +} + +sub isWindowsXP() +{ + return winVersion() eq "5.1"; +} + sub isDarwin() { return ($^O eq "darwin") || 0; @@ -1309,31 +1345,29 @@ sub buildAutotoolsProject($@) } if (! -d $dir) { - system "mkdir", "-p", "$dir"; - if (! -d $dir) { - die "Failed to create build directory " . $dir; - } + File::Path::mkpath($dir) or die "Failed to create build directory " . $dir } - chdir $dir or die "Failed to cd into " . $dir . "\n"; - my $result; if ($clean) { - #$result = system $make, "distclean"; return 0; } - print "Calling configure in " . $dir . "\n\n"; - print "Installation prefix directory: $prefix\n" if(defined($prefix)); - - # Make the path relative since it will appear in all -I compiler flags. - # Long argument lists cause bizarre slowdowns in libtool. - my $relSourceDir = File::Spec->abs2rel($sourceDir); - $relSourceDir = "." if !$relSourceDir; - - $result = system "$relSourceDir/autogen.sh", @buildArgs; - if ($result ne 0) { - die "Failed to setup build environment using 'autotools'!\n"; + # If GNUmakefile exists, don't run autogen.sh. The makefile should be + # smart enough to track autotools dependencies and re-run autogen.sh + # when build files change. + my $result; + if (! -e "GNUmakefile") { + print "Calling configure in " . $dir . "\n\n"; + print "Installation prefix directory: $prefix\n" if(defined($prefix)); + + # Make the path relative since it will appear in all -I compiler flags. + # Long argument lists cause bizarre slowdowns in libtool. + my $relSourceDir = File::Spec->abs2rel($sourceDir) || "."; + $result = system "$relSourceDir/autogen.sh", @buildArgs; + if ($result ne 0) { + die "Failed to setup build environment using 'autotools'!\n"; + } } $result = system "$make $makeArgs"; diff --git a/WebKitTools/Scripts/webkitperl/VCSUtils_unittest/fixChangeLogPatch.pl b/WebKitTools/Scripts/webkitperl/VCSUtils_unittest/fixChangeLogPatch.pl index d21c706..2fd187b 100644 --- a/WebKitTools/Scripts/webkitperl/VCSUtils_unittest/fixChangeLogPatch.pl +++ b/WebKitTools/Scripts/webkitperl/VCSUtils_unittest/fixChangeLogPatch.pl @@ -30,7 +30,7 @@ # Unit tests of VCSUtils::fixChangeLogPatch(). -use Test::Simple tests => 7; +use Test::Simple tests => 8; use VCSUtils; # The source ChangeLog for these tests is the following: @@ -123,6 +123,31 @@ END ok(fixChangeLogPatch($in) eq $in, $title); # New test +$title = "fixChangeLogPatch: [no change] New entry inserted earlier in the file, but after an entry with the same author and date."; + +$in = <<'END'; +--- ChangeLog ++++ ChangeLog +@@ -70,6 +70,14 @@ + + 2009-12-22 Alice <alice@email.address> + ++ Reviewed by Sue. ++ ++ Changed some more code on 2009-12-22. ++ ++ * File: ++ ++2009-12-22 Alice <alice@email.address> ++ + Reviewed by Ray. + + Changed some code on 2009-12-22. +END + +ok(fixChangeLogPatch($in) eq $in, $title); + +# New test $title = "fixChangeLogPatch: Leading context includes first line."; $in = <<'END'; diff --git a/WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl b/WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl index 6af7da4..245916c 100644 --- a/WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl +++ b/WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl @@ -91,10 +91,6 @@ Index: test_file.swf Cannot display: file marked as a binary type. svn:mime-type = application/octet-stream -Property changes on: test_file.swf -___________________________________________________________________ -Name: svn:mime-type - + application/octet-stream Q1dTBx0AAAB42itg4GlgYJjGwMDDyODMxMDw34GBgQEAJPQDJA== @@ -236,6 +232,401 @@ undef], expectedNextLine => undef, }, #### +# Property Changes: Simple +## +{ + # New test + diffName => "SVN: file change diff with property change diff", + inputText => <<'END', +Index: Makefile +=================================================================== +--- Makefile (revision 60021) ++++ Makefile (working copy) +@@ -1,3 +1,4 @@ ++ + MODULES = JavaScriptCore JavaScriptGlue WebCore WebKit WebKit2 WebKitTools + + all: + +Property changes on: Makefile +___________________________________________________________________ +Name: svn:executable + + * +END + expectedReturn => [ +[{ + svnConvertedText => <<'END', # Same as input text +Index: Makefile +=================================================================== +--- Makefile (revision 60021) ++++ Makefile (working copy) +@@ -1,3 +1,4 @@ ++ + MODULES = JavaScriptCore JavaScriptGlue WebCore WebKit WebKit2 WebKitTools + + all: + +END + executableBitDelta => 1, + indexPath => "Makefile", + isSvn => 1, + sourceRevision => "60021", +}], +undef], + expectedNextLine => undef, +}, +{ + # New test + diffName => "SVN: file change diff, followed by property change diff on different file", + inputText => <<'END', +Index: Makefile +=================================================================== +--- Makefile (revision 60021) ++++ Makefile (working copy) +@@ -1,3 +1,4 @@ ++ + MODULES = JavaScriptCore JavaScriptGlue WebCore WebKit WebKit2 WebKitTools + + all: + +Property changes on: Makefile.shared +___________________________________________________________________ +Name: svn:executable + + * +END + expectedReturn => [ +[{ + svnConvertedText => <<'END', # Same as input text +Index: Makefile +=================================================================== +--- Makefile (revision 60021) ++++ Makefile (working copy) +@@ -1,3 +1,4 @@ ++ + MODULES = JavaScriptCore JavaScriptGlue WebCore WebKit WebKit2 WebKitTools + + all: + +END + indexPath => "Makefile", + isSvn => 1, + sourceRevision => "60021", +}], +"Property changes on: Makefile.shared\n"], + expectedNextLine => "___________________________________________________________________\n", +}, +{ + # New test + diffName => "SVN: property diff, followed by file change diff", + inputText => <<'END', +Property changes on: Makefile +___________________________________________________________________ +Deleted: svn:executable + - * + +Index: Makefile.shared +=================================================================== +--- Makefile.shared (revision 60021) ++++ Makefile.shared (working copy) +@@ -1,3 +1,4 @@ ++ +SCRIPTS_PATH ?= ../WebKitTools/Scripts +XCODE_OPTIONS = `perl -I$(SCRIPTS_PATH) -Mwebkitdirs -e 'print XcodeOptionString()'` $(ARGS) +END + expectedReturn => [ +[{ + executableBitDelta => -1, + indexPath => "Makefile", + isSvn => 1, +}], +"Index: Makefile.shared\n"], + expectedNextLine => "===================================================================\n", +}, +{ + # New test + diffName => "SVN: copied file with property change", + inputText => <<'END', +Index: NMakefile +=================================================================== +--- NMakefile (revision 60021) (from Makefile:60021) ++++ NMakefile (working copy) +@@ -0,0 +1,1 @@ ++MODULES = JavaScriptCore JavaScriptGlue WebCore WebKit WebKit2 WebKitTools + +Property changes on: NMakefile +___________________________________________________________________ +Added: svn:executable + + * +END + expectedReturn => [ +[{ + copiedFromPath => "Makefile", + executableBitDelta => 1, + indexPath => "NMakefile", + sourceRevision => "60021", +}], +undef], + expectedNextLine => undef, +}, +{ + # New test + diffName => "SVN: two consecutive property diffs", + inputText => <<'END', +Property changes on: Makefile +___________________________________________________________________ +Added: svn:executable + + * + + +Property changes on: Makefile.shared +___________________________________________________________________ +Added: svn:executable + + * +END + expectedReturn => [ +[{ + executableBitDelta => 1, + indexPath => "Makefile", + isSvn => 1, +}], +"Property changes on: Makefile.shared\n"], + expectedNextLine => "___________________________________________________________________\n", +}, +#### +# Property Changes: Binary files +## +{ + # New test + diffName => "SVN: binary file with executable bit change", + inputText => <<'END', +Index: test_file.swf +=================================================================== +Cannot display: file marked as a binary type. +svn:mime-type = application/octet-stream + +Property changes on: test_file.swf +___________________________________________________________________ +Name: svn:mime-type + + application/octet-stream +Name: svn:executable + + * + + +Q1dTBx0AAAB42itg4GlgYJjGwMDDyODMxMDw34GBgQEAJPQDJA== +END + expectedReturn => [ +[{ + svnConvertedText => <<'END', # Same as input text +Index: test_file.swf +=================================================================== +Cannot display: file marked as a binary type. +svn:mime-type = application/octet-stream + + + +Q1dTBx0AAAB42itg4GlgYJjGwMDDyODMxMDw34GBgQEAJPQDJA== +END + executableBitDelta => 1, + indexPath => "test_file.swf", + isBinary => 1, + isSvn => 1, +}], +undef], + expectedNextLine => undef, +}, +{ + # New test + diffName => "SVN: binary file followed by property change on different file", + inputText => <<'END', +Index: test_file.swf +=================================================================== +Cannot display: file marked as a binary type. +svn:mime-type = application/octet-stream + +Property changes on: test_file.swf +___________________________________________________________________ +Name: svn:mime-type + + application/octet-stream + + +Q1dTBx0AAAB42itg4GlgYJjGwMDDyODMxMDw34GBgQEAJPQDJA== + +Property changes on: Makefile +___________________________________________________________________ +Added: svn:executable + + * +END + expectedReturn => [ +[{ + svnConvertedText => <<'END', # Same as input text +Index: test_file.swf +=================================================================== +Cannot display: file marked as a binary type. +svn:mime-type = application/octet-stream + + + +Q1dTBx0AAAB42itg4GlgYJjGwMDDyODMxMDw34GBgQEAJPQDJA== + +END + indexPath => "test_file.swf", + isBinary => 1, + isSvn => 1, +}], +"Property changes on: Makefile\n"], + expectedNextLine => "___________________________________________________________________\n", +}, +{ + # New test + diffName => "SVN: binary file followed by file change on different file", + inputText => <<'END', +Index: test_file.swf +=================================================================== +Cannot display: file marked as a binary type. +svn:mime-type = application/octet-stream + +Property changes on: test_file.swf +___________________________________________________________________ +Name: svn:mime-type + + application/octet-stream + + +Q1dTBx0AAAB42itg4GlgYJjGwMDDyODMxMDw34GBgQEAJPQDJA== + +Index: Makefile +=================================================================== +--- Makefile (revision 60021) ++++ Makefile (working copy) +@@ -1,3 +1,4 @@ ++ + MODULES = JavaScriptCore JavaScriptGlue WebCore WebKit WebKit2 WebKitTools + + all: +END + expectedReturn => [ +[{ + svnConvertedText => <<'END', # Same as input text +Index: test_file.swf +=================================================================== +Cannot display: file marked as a binary type. +svn:mime-type = application/octet-stream + + + +Q1dTBx0AAAB42itg4GlgYJjGwMDDyODMxMDw34GBgQEAJPQDJA== + +END + indexPath => "test_file.swf", + isBinary => 1, + isSvn => 1, +}], +"Index: Makefile\n"], + expectedNextLine => "===================================================================\n", +}, +#### +# Property Changes: File change with property change +## +{ + # New test + diffName => "SVN: file change diff with property change, followed by property change diff", + inputText => <<'END', +Index: Makefile +=================================================================== +--- Makefile (revision 60021) ++++ Makefile (working copy) +@@ -1,3 +1,4 @@ ++ + MODULES = JavaScriptCore JavaScriptGlue WebCore WebKit WebKit2 WebKitTools + + all: + +Property changes on: Makefile +___________________________________________________________________ +Added: svn:executable + + * + + +Property changes on: Makefile.shared +___________________________________________________________________ +Deleted: svn:executable + - * +END + expectedReturn => [ +[{ + svnConvertedText => <<'END', # Same as input text +Index: Makefile +=================================================================== +--- Makefile (revision 60021) ++++ Makefile (working copy) +@@ -1,3 +1,4 @@ ++ + MODULES = JavaScriptCore JavaScriptGlue WebCore WebKit WebKit2 WebKitTools + + all: + + + +END + executableBitDelta => 1, + indexPath => "Makefile", + isSvn => 1, + sourceRevision => "60021", +}], +"Property changes on: Makefile.shared\n"], + expectedNextLine => "___________________________________________________________________\n", +}, +{ + # New test + diffName => "SVN: file change diff with property change, followed by file change diff", + inputText => <<'END', +Index: Makefile +=================================================================== +--- Makefile (revision 60021) ++++ Makefile (working copy) +@@ -1,3 +1,4 @@ ++ + MODULES = JavaScriptCore JavaScriptGlue WebCore WebKit WebKit2 WebKitTools + + all: + +Property changes on: Makefile +___________________________________________________________________ +Name: svn:executable + - * + +Index: Makefile.shared +=================================================================== +--- Makefile.shared (revision 60021) ++++ Makefile.shared (working copy) +@@ -1,3 +1,4 @@ ++ +SCRIPTS_PATH ?= ../WebKitTools/Scripts +XCODE_OPTIONS = `perl -I$(SCRIPTS_PATH) -Mwebkitdirs -e 'print XcodeOptionString()'` $(ARGS) +END + expectedReturn => [ +[{ + svnConvertedText => <<'END', # Same as input text +Index: Makefile +=================================================================== +--- Makefile (revision 60021) ++++ Makefile (working copy) +@@ -1,3 +1,4 @@ ++ + MODULES = JavaScriptCore JavaScriptGlue WebCore WebKit WebKit2 WebKitTools + + all: + + +END + executableBitDelta => -1, + indexPath => "Makefile", + isSvn => 1, + sourceRevision => "60021", +}], +"Index: Makefile.shared\n"], + expectedNextLine => "===================================================================\n", +}, +#### # Git test cases ## { @@ -451,6 +842,7 @@ END indexPath => "foo_new", }, { + executableBitDelta => 1, indexPath => "foo_new", isGit => 1, svnConvertedText => <<'END', diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/api.py b/WebKitTools/Scripts/webkitpy/common/checkout/api.py index a5ac939..ca28e32 100644 --- a/WebKitTools/Scripts/webkitpy/common/checkout/api.py +++ b/WebKitTools/Scripts/webkitpy/common/checkout/api.py @@ -83,16 +83,16 @@ class Checkout(object): def bug_id_for_revision(self, revision): return self.commit_info_for_revision(revision).bug_id() - def modified_changelogs(self, git_commit, squash): + def modified_changelogs(self, git_commit): # SCM returns paths relative to scm.checkout_root # Callers (especially those using the ChangeLog class) may # expect absolute paths, so this method returns absolute paths. - changed_files = self._scm.changed_files(git_commit, squash) + changed_files = self._scm.changed_files(git_commit) absolute_paths = [os.path.join(self._scm.checkout_root, path) for path in changed_files] return [path for path in absolute_paths if self._is_path_to_changelog(path)] - def commit_message_for_this_commit(self, git_commit, squash): - changelog_paths = self.modified_changelogs(git_commit, squash) + def commit_message_for_this_commit(self, git_commit): + changelog_paths = self.modified_changelogs(git_commit) if not len(changelog_paths): raise ScriptError(message="Found no modified ChangeLogs, cannot create a commit message.\n" "All changes require a ChangeLog. See:\n" @@ -109,9 +109,9 @@ class Checkout(object): # FIXME: We should sort and label the ChangeLog messages like commit-log-editor does. return CommitMessage("".join(changelog_messages).splitlines()) - def bug_id_for_this_commit(self, git_commit, squash): + def bug_id_for_this_commit(self, git_commit): try: - return parse_bug_id(self.commit_message_for_this_commit(git_commit, squash).message()) + return parse_bug_id(self.commit_message_for_this_commit(git_commit).message()) except ScriptError, e: pass # We might not have ChangeLogs. @@ -131,7 +131,7 @@ class Checkout(object): # We revert the ChangeLogs because removing lines from a ChangeLog # doesn't make sense. ChangeLogs are append only. - changelog_paths = self.modified_changelogs(git_commit=None, squash=False) + changelog_paths = self.modified_changelogs(git_commit=None) if len(changelog_paths): self._scm.revert_files(changelog_paths) diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/api_unittest.py b/WebKitTools/Scripts/webkitpy/common/checkout/api_unittest.py index 1436379..fdfd879 100644 --- a/WebKitTools/Scripts/webkitpy/common/checkout/api_unittest.py +++ b/WebKitTools/Scripts/webkitpy/common/checkout/api_unittest.py @@ -114,11 +114,11 @@ class CommitMessageForThisCommitTest(unittest.TestCase): # ChangeLog is difficult to mock at current. def test_commit_message_for_this_commit(self): checkout = Checkout(None) - checkout.modified_changelogs = lambda git_commit, squash: ["ChangeLog1", "ChangeLog2"] + checkout.modified_changelogs = lambda git_commit: ["ChangeLog1", "ChangeLog2"] output = OutputCapture() expected_stderr = "Parsing ChangeLog: ChangeLog1\nParsing ChangeLog: ChangeLog2\n" commit_message = output.assert_outputs(self, checkout.commit_message_for_this_commit, - kwargs={"git_commit": None, "squash": False}, expected_stderr=expected_stderr) + kwargs={"git_commit": None}, expected_stderr=expected_stderr) self.assertEqual(commit_message.message(), self.expected_commit_message) @@ -163,13 +163,13 @@ class CheckoutTest(unittest.TestCase): def test_bug_id_for_this_commit(self): scm = Mock() checkout = Checkout(scm) - checkout.commit_message_for_this_commit = lambda git_commit, squash: CommitMessage(ChangeLogEntry(_changelog1entry1).contents().splitlines()) - self.assertEqual(checkout.bug_id_for_this_commit(git_commit=None, squash=False), 36629) + checkout.commit_message_for_this_commit = lambda git_commit: CommitMessage(ChangeLogEntry(_changelog1entry1).contents().splitlines()) + self.assertEqual(checkout.bug_id_for_this_commit(git_commit=None), 36629) def test_modified_changelogs(self): scm = Mock() scm.checkout_root = "/foo/bar" - scm.changed_files = lambda git_commit, squash: ["file1", "ChangeLog", "relative/path/ChangeLog"] + scm.changed_files = lambda git_commit: ["file1", "ChangeLog", "relative/path/ChangeLog"] checkout = Checkout(scm) expected_changlogs = ["/foo/bar/ChangeLog", "/foo/bar/relative/path/ChangeLog"] - self.assertEqual(checkout.modified_changelogs(git_commit=None, squash=False), expected_changlogs) + self.assertEqual(checkout.modified_changelogs(git_commit=None), expected_changlogs) diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/scm.py b/WebKitTools/Scripts/webkitpy/common/checkout/scm.py index d7c621c..569558a 100644 --- a/WebKitTools/Scripts/webkitpy/common/checkout/scm.py +++ b/WebKitTools/Scripts/webkitpy/common/checkout/scm.py @@ -35,7 +35,6 @@ import sys import shutil from webkitpy.common.system.executive import Executive, run_command, ScriptError -from webkitpy.common.system.user import User from webkitpy.common.system.deprecated_logging import error, log @@ -94,6 +93,17 @@ def commit_error_handler(error): Executive.default_error_handler(error) +class AuthenticationError(Exception): + def __init__(self, server_host): + self.server_host = server_host + + +class AmbiguousCommitError(Exception): + def __init__(self, num_local_commits, working_directory_is_clean): + self.num_local_commits = num_local_commits + self.working_directory_is_clean = working_directory_is_clean + + # SCM methods are expected to return paths relative to self.checkout_root. class SCM: def __init__(self, cwd): @@ -198,7 +208,7 @@ class SCM: def delete(self, path): self._subclass_must_implement() - def changed_files(self, git_commit=None, squash=None): + def changed_files(self, git_commit=None): self._subclass_must_implement() def changed_files_for_revision(self): @@ -213,7 +223,7 @@ class SCM: def display_name(self): self._subclass_must_implement() - def create_patch(self, git_commit=None, squash=None): + def create_patch(self, git_commit=None): self._subclass_must_implement() def committer_email_for_revision(self, revision): @@ -237,10 +247,7 @@ class SCM: def revert_files(self, file_paths): self._subclass_must_implement() - def should_squash(self, squash): - self._subclass_must_implement() - - def commit_with_message(self, message, username=None, git_commit=None, squash=None): + def commit_with_message(self, message, username=None, git_commit=None, force_squash=False): self._subclass_must_implement() def svn_commit_log(self, svn_revision): @@ -377,7 +384,7 @@ class SVN(SCM): parent, base = os.path.split(os.path.abspath(path)) return self.run(["svn", "delete", "--force", base], cwd=parent) - def changed_files(self, git_commit=None, squash=None): + def changed_files(self, git_commit=None): return self.run_status_and_extract_filenames(self.status_command(), self._status_regexp("ACDMR")) def changed_files_for_revision(self, revision): @@ -403,7 +410,7 @@ class SVN(SCM): return "svn" # FIXME: This method should be on Checkout. - def create_patch(self, git_commit=None, squash=None): + def create_patch(self, git_commit=None): """Returns a byte array (str()) representing the patch file. Patch files are effectively binary since they may contain files of multiple different encodings.""" @@ -477,22 +484,19 @@ class SVN(SCM): # FIXME: This should probably use cwd=self.checkout_root. self.run(['svn', 'revert'] + file_paths) - def should_squash(self, squash): - # SVN doesn't support the concept of squashing. - return False - - def commit_with_message(self, message, username=None, git_commit=None, squash=None): - # squash and git-commit are not used by SVN. + def commit_with_message(self, message, username=None, git_commit=None, force_squash=False): + # git-commit and force are not used by SVN. if self.dryrun: # Return a string which looks like a commit so that things which parse this output will succeed. return "Dry run, no commit.\nCommitted revision 0." + svn_commit_args = ["svn", "commit"] + if not username and not self.has_authorization_for_realm(): - username = User.prompt("%s login: " % self.svn_server_host, repeat=5) - if not username: - raise Exception("You need to specify the username on %s to perform the commit as." % self.svn_server_host) + raise AuthenticationError(self.svn_server_host) if username: svn_commit_args.extend(["--username", username]) + svn_commit_args.extend(["-m", message]) # FIXME: Should this use cwd=self.checkout_root? return self.run(svn_commit_args, error_handler=commit_error_handler) @@ -584,24 +588,25 @@ class Git(SCM): def delete(self, path): return self.run(["git", "rm", "-f", path]) - def _merge_base(self, git_commit, squash): + def _assert_synced(self): + if len(run_command(['git', 'rev-list', '--max-count=1', self.remote_branch_ref(), '^HEAD'])): + raise ScriptError(message="Not fully merged/rebased to %s. This branch needs to be synced first." % self.remote_branch_ref()) + + def merge_base(self, git_commit): if git_commit: - # FIXME: Calling code should turn commit ranges into a list of commit IDs - # and then treat each commit separately. + # Special-case HEAD.. to mean working-copy changes only. + if git_commit.upper() == 'HEAD..': + return 'HEAD' + if '..' not in git_commit: git_commit = git_commit + "^.." + git_commit return git_commit - if self.should_squash(squash): - return self.remote_merge_base() - - # FIXME: Non-squash behavior should match commit_with_message. It raises an error - # if there are working copy changes and --squash or --no-squash wasn't passed in. - # If --no-squash, then it should proceed with each local commit as a separate patch. - return 'HEAD' + self._assert_synced() + return self.remote_merge_base() - def changed_files(self, git_commit=None, squash=None): - status_command = ['git', 'diff', '-r', '--name-status', '-C', '-M', "--no-ext-diff", "--full-index", self._merge_base(git_commit, squash)] + def changed_files(self, git_commit=None): + status_command = ['git', 'diff', '-r', '--name-status', '-C', '-M', "--no-ext-diff", "--full-index", self.merge_base(git_commit)] return self.run_status_and_extract_filenames(status_command, self._status_regexp("ADM")) def _changes_files_for_commit(self, git_commit): @@ -633,12 +638,12 @@ class Git(SCM): def display_name(self): return "git" - def create_patch(self, git_commit=None, squash=None): + def create_patch(self, git_commit=None): """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, squash)], decode_output=False) + return self.run(['git', 'diff', '--binary', "--no-ext-diff", "--full-index", "-M", self.merge_base(git_commit)], decode_output=False) @classmethod def git_commit_from_svn_revision(cls, revision): @@ -679,63 +684,41 @@ class Git(SCM): def revert_files(self, file_paths): self.run(['git', 'checkout', 'HEAD'] + file_paths) - def _get_squash_error_message(self, num_local_commits): - working_directory_message = "" if self.working_directory_is_clean() else " and working copy changes" - return ("""There are %s local commits%s. Do one of the following: - 1) Use --squash or --no-squash - 2) git config webkit-patch.squash true/false - """ % (num_local_commits, working_directory_message)) - - def should_squash(self, squash): - if squash is None: - config_squash = Git.read_git_config('webkit-patch.squash') - if (config_squash and config_squash is not ""): - squash = config_squash.lower() == "true" - else: - # Only raise an error if there are actually multiple commits to squash. - num_local_commits = len(self.local_commits()) - if num_local_commits > 1 or (num_local_commits > 0 and not self.working_directory_is_clean()): - raise ScriptError(message=self._get_squash_error_message(num_local_commits)) - - if squash and self._remote_branch_has_extra_commits(): - raise ScriptError(message="Cannot use --squash when HEAD is not fully merged/rebased to %s. " - "This branch needs to be synced first." % self.remote_branch_ref()) + def _assert_can_squash(self, working_directory_is_clean): + squash = Git.read_git_config('webkit-patch.commit_should_always_squash') + should_squash = squash and squash.lower() == "true" - return squash + if not should_squash: + # Only warn if there are actually multiple commits to squash. + num_local_commits = len(self.local_commits()) + if num_local_commits > 1 or (num_local_commits > 0 and not working_directory_is_clean): + raise AmbiguousCommitError(num_local_commits, working_directory_is_clean) - def _remote_branch_has_extra_commits(self): - return len(run_command(['git', 'rev-list', '--max-count=1', self.remote_branch_ref(), '^HEAD'])) - - def commit_with_message(self, message, username=None, git_commit=None, squash=None): + def commit_with_message(self, message, username=None, git_commit=None, force_squash=False): # Username is ignored during Git commits. + working_directory_is_clean = self.working_directory_is_clean() + if git_commit: + # Special-case HEAD.. to mean working-copy changes only. + if git_commit.upper() == 'HEAD..': + if working_directory_is_clean: + raise ScriptError(message="The working copy is not modified. --git-commit=HEAD.. only commits working copy changes.") + self.commit_locally_with_message(message) + return self._commit_on_branch(message, 'HEAD') + # Need working directory changes to be committed so we can checkout the merge branch. - if not self.working_directory_is_clean(): + if not working_directory_is_clean: # FIXME: webkit-patch land will modify the ChangeLogs to correct the reviewer. # That will modify the working-copy and cause us to hit this error. - # The ChangeLog modification could be made to modify the existing local commit? + # The ChangeLog modification could be made to modify the existing local commit. raise ScriptError(message="Working copy is modified. Cannot commit individual git_commits.") return self._commit_on_branch(message, git_commit) - squash = self.should_squash(squash) - if squash: - self.run(['git', 'reset', '--soft', self.remote_branch_ref()]) - self.commit_locally_with_message(message) - elif not self.working_directory_is_clean(): - if not len(self.local_commits()): - # There are only working copy changes. Assume they should be committed. - self.commit_locally_with_message(message) - elif squash is None: - # The user didn't explicitly say to squash or not squash. There are local commits - # and working copy changes. Not clear what the user wants. - raise ScriptError(message="""There are local commits and working copy changes. Do one of the following: -1) Commit/revert working copy changes. -2) Use --squash or --no-squash -3) git config webkit-patch.squash true/false -""") - - # FIXME: This will commit all local commits, each with it's own message. We should restructure - # so that each local commit has the appropriate commit message based off it's ChangeLogs. + if not force_squash: + self._assert_can_squash(working_directory_is_clean) + self._assert_synced() + self.run(['git', 'reset', '--soft', self.remote_branch_ref()]) + self.commit_locally_with_message(message) return self.push_local_commits_to_server() def _commit_on_branch(self, message, git_commit): diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/scm_unittest.py b/WebKitTools/Scripts/webkitpy/common/checkout/scm_unittest.py index eaa3b46..852f838 100644 --- a/WebKitTools/Scripts/webkitpy/common/checkout/scm_unittest.py +++ b/WebKitTools/Scripts/webkitpy/common/checkout/scm_unittest.py @@ -44,7 +44,7 @@ import shutil from datetime import date from webkitpy.common.checkout.api import Checkout -from webkitpy.common.checkout.scm import detect_scm_system, SCM, SVN, CheckoutNeedsUpdate, commit_error_handler +from webkitpy.common.checkout.scm import detect_scm_system, SCM, SVN, CheckoutNeedsUpdate, commit_error_handler, AuthenticationError, AmbiguousCommitError from webkitpy.common.config.committers import Committer # FIXME: This should not be needed from webkitpy.common.net.bugzilla import Attachment # FIXME: This should not be needed from webkitpy.common.system.executive import Executive, run_command, ScriptError @@ -597,6 +597,10 @@ Q1dTBx0AAAB42itg4GlgYJjGwMDDyODMxMDw34GBgQEAJPQDJA== def test_commit_with_username(self): self._shared_test_commit_with_message("dbates@webkit.org") + def test_commit_without_authorization(self): + self.scm.has_authorization_for_realm = lambda: False + self.assertRaises(AuthenticationError, self._shared_test_commit_with_message) + def test_has_authorization_for_realm(self): scm = detect_scm_system(self.svn_checkout_path) fake_home_dir = tempfile.mkdtemp(suffix="fake_home_dir") @@ -867,24 +871,14 @@ class GitSVNTest(SCMTest): def test_commit_text_parsing(self): write_into_file_at_path('test_file', 'more test content') - self.scm.commit_locally_with_message("another test commit") commit_text = self.scm.commit_with_message("another test commit") self.assertEqual(self.scm.svn_revision_from_commit_text(commit_text), '6') self.scm.dryrun = True write_into_file_at_path('test_file', 'still more test content') - self.scm.commit_locally_with_message("yet another test commit") commit_text = self.scm.commit_with_message("yet another test commit") self.assertEqual(self.scm.svn_revision_from_commit_text(commit_text), '0') - def _one_local_commit_plus_working_copy_changes(self): - write_into_file_at_path('test_file_commit1', 'more test content') - run_command(['git', 'add', 'test_file_commit1']) - self.scm.commit_locally_with_message("another test commit") - - write_into_file_at_path('test_file_commit2', 'still more test content') - run_command(['git', 'add', 'test_file_commit2']) - def test_commit_with_message_working_copy_only(self): write_into_file_at_path('test_file_commit1', 'more test content') run_command(['git', 'add', 'test_file_commit1']) @@ -895,21 +889,18 @@ class GitSVNTest(SCMTest): svn_log = run_command(['git', 'svn', 'log', '--limit=1', '--verbose']) self.assertTrue(re.search(r'test_file_commit1', svn_log)) - def test_commit_with_message_squashed(self): - self._one_local_commit_plus_working_copy_changes() - scm = detect_scm_system(self.git_checkout_path) - commit_text = scm.commit_with_message("yet another test commit", squash=True) - - self.assertEqual(scm.svn_revision_from_commit_text(commit_text), '6') - svn_log = run_command(['git', 'svn', 'log', '--limit=1', '--verbose']) - self.assertTrue(re.search(r'test_file_commit2', svn_log)) - self.assertTrue(re.search(r'test_file_commit1', svn_log)) - - def _two_local_commits(self): + def _one_local_commit(self): write_into_file_at_path('test_file_commit1', 'more test content') run_command(['git', 'add', 'test_file_commit1']) self.scm.commit_locally_with_message("another test commit") + def _one_local_commit_plus_working_copy_changes(self): + self._one_local_commit() + write_into_file_at_path('test_file_commit2', 'still more test content') + run_command(['git', 'add', 'test_file_commit2']) + + def _two_local_commits(self): + self._one_local_commit() write_into_file_at_path('test_file_commit2', 'still more test content') run_command(['git', 'add', 'test_file_commit2']) self.scm.commit_locally_with_message("yet another test commit") @@ -920,6 +911,17 @@ class GitSVNTest(SCMTest): self.scm.commit_locally_with_message("another test commit") self._two_local_commits() + def test_commit_with_message(self): + self._one_local_commit_plus_working_copy_changes() + scm = detect_scm_system(self.git_checkout_path) + self.assertRaises(AmbiguousCommitError, scm.commit_with_message, "yet another test commit") + commit_text = scm.commit_with_message("yet another test commit", force_squash=True) + + self.assertEqual(scm.svn_revision_from_commit_text(commit_text), '6') + svn_log = run_command(['git', 'svn', 'log', '--limit=1', '--verbose']) + self.assertTrue(re.search(r'test_file_commit2', svn_log)) + self.assertTrue(re.search(r'test_file_commit1', svn_log)) + def test_commit_with_message_git_commit(self): self._two_local_commits() @@ -943,51 +945,68 @@ class GitSVNTest(SCMTest): self.assertTrue(re.search(r'test_file_commit1', svn_log)) self.assertTrue(re.search(r'test_file_commit2', svn_log)) - def test_commit_with_message_multiple_local_commits(self): - self._two_local_commits() + def test_changed_files_working_copy_only(self): + self._one_local_commit_plus_working_copy_changes() scm = detect_scm_system(self.git_checkout_path) - self.assertRaises(ScriptError, scm.commit_with_message, ["another test commit"]) + commit_text = scm.commit_with_message("another test commit", git_commit="HEAD..") + self.assertFalse(re.search(r'test_file_commit1', svn_log)) + self.assertTrue(re.search(r'test_file_commit2', svn_log)) + + def test_commit_with_message_only_local_commit(self): + self._one_local_commit() + scm = detect_scm_system(self.git_checkout_path) + commit_text = scm.commit_with_message("another test commit") + svn_log = run_command(['git', 'svn', 'log', '--limit=1', '--verbose']) + self.assertTrue(re.search(r'test_file_commit1', svn_log)) def test_commit_with_message_multiple_local_commits_and_working_copy(self): self._two_local_commits() write_into_file_at_path('test_file_commit1', 'working copy change') scm = detect_scm_system(self.git_checkout_path) - self.assertRaises(ScriptError, scm.commit_with_message, ["another test commit"]) + + self.assertRaises(AmbiguousCommitError, scm.commit_with_message, "another test commit") + commit_text = scm.commit_with_message("another test commit", force_squash=True) + + self.assertEqual(scm.svn_revision_from_commit_text(commit_text), '6') + svn_log = run_command(['git', 'svn', 'log', '--limit=1', '--verbose']) + self.assertTrue(re.search(r'test_file_commit2', svn_log)) + self.assertTrue(re.search(r'test_file_commit1', svn_log)) def test_commit_with_message_git_commit_and_working_copy(self): self._two_local_commits() write_into_file_at_path('test_file_commit1', 'working copy change') scm = detect_scm_system(self.git_checkout_path) - self.assertRaises(ScriptError, scm.commit_with_message, ["another test commit", 'git_commit="HEAD^"']) + self.assertRaises(ScriptError, scm.commit_with_message, "another test commit", git_commit="HEAD^") - def test_commit_with_message_multiple_local_commits_no_squash(self): + def test_commit_with_message_multiple_local_commits_always_squash(self): self._two_local_commits() scm = detect_scm_system(self.git_checkout_path) - commit_text = scm.commit_with_message("yet another test commit", squash=False) + scm._assert_can_squash = lambda working_directory_is_clean: True + commit_text = scm.commit_with_message("yet another test commit") self.assertEqual(scm.svn_revision_from_commit_text(commit_text), '6') svn_log = run_command(['git', 'svn', 'log', '--limit=1', '--verbose']) self.assertTrue(re.search(r'test_file_commit2', svn_log)) - self.assertFalse(re.search(r'test_file_commit1', svn_log)) - - svn_log = run_command(['git', 'svn', 'log', '--limit=2', '--verbose']) self.assertTrue(re.search(r'test_file_commit1', svn_log)) - def test_commit_with_message_multiple_local_commits_squash(self): + def test_commit_with_message_multiple_local_commits(self): self._two_local_commits() scm = detect_scm_system(self.git_checkout_path) - commit_text = scm.commit_with_message("yet another test commit", squash=True) + self.assertRaises(AmbiguousCommitError, scm.commit_with_message, "yet another test commit") + commit_text = scm.commit_with_message("yet another test commit", force_squash=True) + self.assertEqual(scm.svn_revision_from_commit_text(commit_text), '6') svn_log = run_command(['git', 'svn', 'log', '--limit=1', '--verbose']) self.assertTrue(re.search(r'test_file_commit2', svn_log)) self.assertTrue(re.search(r'test_file_commit1', svn_log)) - def test_commit_with_message_not_synced_squash(self): + def test_commit_with_message_not_synced(self): run_command(['git', 'checkout', '-b', 'my-branch', 'trunk~3']) self._two_local_commits() scm = detect_scm_system(self.git_checkout_path) - self.assertRaises(ScriptError, scm.commit_with_message, "another test commit", squash=True) + self.assertRaises(AmbiguousCommitError, scm.commit_with_message, "another test commit") + self.assertRaises(ScriptError, scm.commit_with_message, "another test commit", force_squash=True) def test_remote_branch_ref(self): self.assertEqual(self.scm.remote_branch_ref(), 'refs/remotes/trunk') @@ -1004,26 +1023,16 @@ class GitSVNTest(SCMTest): def test_create_patch_local_plus_working_copy(self): self._one_local_commit_plus_working_copy_changes() scm = detect_scm_system(self.git_checkout_path) - self.assertRaises(ScriptError, scm.create_patch) - - def test_create_patch_multiple_local_commits(self): - self._two_local_commits() - scm = detect_scm_system(self.git_checkout_path) - self.assertRaises(ScriptError, scm.create_patch) - - def test_create_patch_squashed(self): - self._one_local_commit_plus_working_copy_changes() - scm = detect_scm_system(self.git_checkout_path) - patch = scm.create_patch(squash=True) - self.assertTrue(re.search(r'test_file_commit2', patch)) + patch = scm.create_patch() self.assertTrue(re.search(r'test_file_commit1', patch)) + self.assertTrue(re.search(r'test_file_commit2', patch)) - def test_create_patch_not_squashed(self): + def test_create_patch(self): self._one_local_commit_plus_working_copy_changes() scm = detect_scm_system(self.git_checkout_path) - patch = scm.create_patch(squash=False) + patch = scm.create_patch() self.assertTrue(re.search(r'test_file_commit2', patch)) - self.assertFalse(re.search(r'test_file_commit1', patch)) + self.assertTrue(re.search(r'test_file_commit1', patch)) def test_create_patch_git_commit(self): self._two_local_commits() @@ -1040,26 +1049,25 @@ class GitSVNTest(SCMTest): self.assertTrue(re.search(r'test_file_commit2', patch)) self.assertTrue(re.search(r'test_file_commit1', patch)) - def test_create_patch_multiple_local_commits_no_squash(self): - self._two_local_commits() + def test_create_patch_working_copy_only(self): + self._one_local_commit_plus_working_copy_changes() scm = detect_scm_system(self.git_checkout_path) - patch = scm.create_patch(squash=False) - # FIXME: It's weird that with squash=False, create_patch/changed_files ignores local commits, - # but commit_with_message commits them. - self.assertTrue(patch == "") + patch = scm.create_patch(git_commit="HEAD..") + self.assertFalse(re.search(r'test_file_commit1', patch)) + self.assertTrue(re.search(r'test_file_commit2', patch)) - def test_create_patch_multiple_local_commits_squash(self): + def test_create_patch_multiple_local_commits(self): self._two_local_commits() scm = detect_scm_system(self.git_checkout_path) - patch = scm.create_patch(squash=True) + patch = scm.create_patch() self.assertTrue(re.search(r'test_file_commit2', patch)) self.assertTrue(re.search(r'test_file_commit1', patch)) - def test_create_patch_not_synced_squash(self): + def test_create_patch_not_synced(self): run_command(['git', 'checkout', '-b', 'my-branch', 'trunk~3']) self._two_local_commits() scm = detect_scm_system(self.git_checkout_path) - self.assertRaises(ScriptError, scm.create_patch, squash=True) + self.assertRaises(ScriptError, scm.create_patch) def test_create_binary_patch(self): # Create a git binary patch and check the contents. @@ -1090,26 +1098,9 @@ class GitSVNTest(SCMTest): def test_changed_files_local_plus_working_copy(self): self._one_local_commit_plus_working_copy_changes() scm = detect_scm_system(self.git_checkout_path) - self.assertRaises(ScriptError, scm.changed_files) - - def test_changed_files_multiple_local_commits(self): - self._two_local_commits() - scm = detect_scm_system(self.git_checkout_path) - self.assertRaises(ScriptError, scm.changed_files) - - def test_changed_files_squashed(self): - self._one_local_commit_plus_working_copy_changes() - scm = detect_scm_system(self.git_checkout_path) - files = scm.changed_files(squash=True) - self.assertTrue('test_file_commit2' in files) + files = scm.changed_files() self.assertTrue('test_file_commit1' in files) - - def test_changed_files_not_squashed(self): - self._one_local_commit_plus_working_copy_changes() - scm = detect_scm_system(self.git_checkout_path) - files = scm.changed_files(squash=False) self.assertTrue('test_file_commit2' in files) - self.assertFalse('test_file_commit1' in files) def test_changed_files_git_commit(self): self._two_local_commits() @@ -1126,26 +1117,25 @@ class GitSVNTest(SCMTest): self.assertTrue('test_file_commit1' in files) self.assertTrue('test_file_commit2' in files) - def test_changed_files_multiple_local_commits_no_squash(self): - self._two_local_commits() + def test_changed_files_working_copy_only(self): + self._one_local_commit_plus_working_copy_changes() scm = detect_scm_system(self.git_checkout_path) - files = scm.changed_files(squash=False) - # FIXME: It's weird that with squash=False, create_patch/changed_files ignores local commits, - # but commit_with_message commits them. - self.assertTrue(len(files) == 0) + files = scm.changed_files(git_commit="HEAD..") + self.assertFalse('test_file_commit1' in files) + self.assertTrue('test_file_commit2' in files) - def test_changed_files_multiple_local_commits_squash(self): + def test_changed_files_multiple_local_commits(self): self._two_local_commits() scm = detect_scm_system(self.git_checkout_path) - files = scm.changed_files(squash=True) + files = scm.changed_files() self.assertTrue('test_file_commit2' in files) self.assertTrue('test_file_commit1' in files) - def test_changed_files_not_synced_squash(self): + def test_changed_files_not_synced(self): run_command(['git', 'checkout', '-b', 'my-branch', 'trunk~3']) self._two_local_commits() scm = detect_scm_system(self.git_checkout_path) - self.assertRaises(ScriptError, scm.changed_files, squash=True) + self.assertRaises(ScriptError, scm.changed_files) def test_changed_files(self): self._shared_test_changed_files() diff --git a/WebKitTools/Scripts/webkitpy/common/config/committers.py b/WebKitTools/Scripts/webkitpy/common/config/committers.py index 7b69815..e5f0a50 100644 --- a/WebKitTools/Scripts/webkitpy/common/config/committers.py +++ b/WebKitTools/Scripts/webkitpy/common/config/committers.py @@ -64,6 +64,7 @@ class Reviewer(Committer): committers_unable_to_review = [ Committer("Aaron Boodman", "aa@chromium.org", "aboodman"), + Committer("Abhishek Arya", "inferno@chromium.org", "inferno-sec"), Committer("Adam Langley", "agl@chromium.org", "agl"), Committer("Albert J. Wong", "ajwong@chromium.org"), Committer("Alejandro G. Castro", ["alex@igalia.com", "alex@webkit.org"]), @@ -76,7 +77,6 @@ committers_unable_to_review = [ Committer("Andy Estes", "aestes@apple.com", "estes"), Committer("Anthony Ricaud", "rik@webkit.org", "rik"), Committer("Anton Muhin", "antonm@chromium.org", "antonm"), - Committer("Antonio Gomes", "tonikitoo@webkit.org", "tonikitoo"), Committer("Ben Murdoch", "benm@google.com", "benm"), Committer("Benjamin C Meyer", ["ben@meyerhome.net", "ben@webkit.org"], "icefox"), Committer("Benjamin Otte", ["otte@gnome.org", "otte@webkit.org"], "otte"), @@ -115,6 +115,7 @@ committers_unable_to_review = [ Committer("Jakub Wieczorek", "jwieczorek@webkit.org", "fawek"), Committer("James Hawkins", ["jhawkins@chromium.org", "jhawkins@google.com"], "jhawkins"), Committer("James Robinson", ["jamesr@chromium.org", "jamesr@google.com"], "jamesr"), + Committer("Jay Civelli", "jcivelli@chromium.org", "jcivelli"), Committer("Jens Alfke", ["snej@chromium.org", "jens@apple.com"]), Committer("Jer Noble", "jer.noble@apple.com", "jernoble"), Committer("Jeremy Moskovich", ["playmobil@google.com", "jeremy@chromium.org"], "jeremymos"), @@ -149,6 +150,7 @@ committers_unable_to_review = [ Committer("Mike Thole", ["mthole@mikethole.com", "mthole@apple.com"]), Committer("Mikhail Naganov", "mnaganov@chromium.org"), Committer("MORITA Hajime", "morrita@google.com", "morrita"), + Committer("Nico Weber", ["thakis@chromium.org", "thakis@google.com"], "thakis"), Committer("Pam Greene", "pam@chromium.org", "pamg"), Committer("Peter Kasting", ["pkasting@google.com", "pkasting@chromium.org"], "pkasting"), Committer("Philippe Normand", ["pnormand@igalia.com", "philn@webkit.org"], "philn-tp"), @@ -193,6 +195,7 @@ reviewers_list = [ Reviewer("Alice Liu", "alice.liu@apple.com", "aliu"), Reviewer("Alp Toker", ["alp@nuanti.com", "alp@atoker.com", "alp@webkit.org"], "alp"), Reviewer("Anders Carlsson", ["andersca@apple.com", "acarlsson@apple.com"], "andersca"), + Reviewer("Antonio Gomes", "tonikitoo@webkit.org", "tonikitoo"), Reviewer("Antti Koivisto", ["koivisto@iki.fi", "antti@apple.com"], "anttik"), Reviewer("Ariya Hidayat", ["ariya.hidayat@gmail.com", "ariya@webkit.org"], "ariya"), Reviewer("Beth Dakin", "bdakin@apple.com", "dethbakin"), @@ -254,7 +257,7 @@ reviewers_list = [ Reviewer("Steve Falkenburg", "sfalken@apple.com", "sfalken"), Reviewer("Tim Omernick", "timo@apple.com"), Reviewer("Timothy Hatcher", ["timothy@hatcher.name", "timothy@apple.com"], "xenon"), - Reviewer(u"Tor Arne Vestb\u00f8", "vestbo@webkit.org", "torarne"), + Reviewer(u"Tor Arne Vestb\u00f8", ["vestbo@webkit.org", "tor.arne.vestbo@nokia.com"], "torarne"), Reviewer("Vicki Murley", "vicki@apple.com"), Reviewer("Xan Lopez", ["xan.lopez@gmail.com", "xan@gnome.org", "xan@webkit.org"], "xan"), Reviewer("Yury Semikhatsky", "yurys@chromium.org", "yurys"), diff --git a/WebKitTools/Scripts/webkitpy/common/net/rietveld.py b/WebKitTools/Scripts/webkitpy/common/net/rietveld.py index eccda3a..0c6a313 100644 --- a/WebKitTools/Scripts/webkitpy/common/net/rietveld.py +++ b/WebKitTools/Scripts/webkitpy/common/net/rietveld.py @@ -73,6 +73,6 @@ class Rietveld(object): # Use RealMain instead of calling upload from the commandline so that # we can pass in the diff ourselves. Otherwise, upload will just use - # git diff for git checkouts, which doesn't respect --squash and --git-commit. + # git diff for git checkouts, which doesn't respect --git-commit. issue, patchset = upload.RealMain(args, data=diff) return issue diff --git a/WebKitTools/Scripts/webkitpy/common/system/executive.py b/WebKitTools/Scripts/webkitpy/common/system/executive.py index 9c5889b..6088680 100644 --- a/WebKitTools/Scripts/webkitpy/common/system/executive.py +++ b/WebKitTools/Scripts/webkitpy/common/system/executive.py @@ -183,7 +183,7 @@ class Executive(object): # According to http://docs.python.org/library/os.html # os.kill isn't available on Windows. python 2.5.5 os.kill appears # to work in cygwin, however it occasionally raises EAGAIN. - retries_left = 3 if sys.platform == "cygwin" else 1 + retries_left = 10 if sys.platform == "cygwin" else 1 while retries_left > 0: try: retries_left -= 1 diff --git a/WebKitTools/Scripts/webkitpy/common/system/user.py b/WebKitTools/Scripts/webkitpy/common/system/user.py index b4df3cb..9444c00 100644 --- a/WebKitTools/Scripts/webkitpy/common/system/user.py +++ b/WebKitTools/Scripts/webkitpy/common/system/user.py @@ -51,7 +51,7 @@ except ImportError: class User(object): - # FIXME: These are @classmethods because scm.py and bugzilla.py don't have a Tool object (thus no User instance). + # FIXME: These are @classmethods because bugzilla.py doesn't have a Tool object (thus no User instance). @classmethod def prompt(cls, message, repeat=1, raw_input=raw_input): response = None 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 a2e2091..6364511 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 @@ -429,7 +429,8 @@ class TestShellThread(threading.Thread): # previous run will be copied into the baseline.) image_hash = test_info.image_hash() if (image_hash and - (self._test_args.new_baseline or self._test_args.reset_results)): + (self._test_args.new_baseline or self._test_args.reset_results or + not self._options.pixel_tests)): image_hash = "" start = time.time() crash, timeout, actual_checksum, output, error = \ 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 bb214f7..c0525ea 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 @@ -1,4 +1,3 @@ -#!/usr/bin/env python # Copyright (C) 2010 Google Inc. All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -35,7 +34,8 @@ from webkitpy.layout_tests.layout_package import test_expectations from webkitpy.layout_tests.layout_package import test_failures import webkitpy.thirdparty.simplejson as simplejson -class JSONLayoutResultsGenerator(json_results_generator.JSONResultsGenerator): + +class JSONLayoutResultsGenerator(json_results_generator.JSONResultsGeneratorBase): """A JSON results generator for layout tests.""" LAYOUT_TESTS_PATH = "LayoutTests" @@ -44,6 +44,16 @@ class JSONLayoutResultsGenerator(json_results_generator.JSONResultsGenerator): WONTFIX = "wontfixCounts" DEFERRED = "deferredCounts" + # Note that we omit test_expectations.FAIL from this list because + # it should never show up (it's a legacy input expectation, never + # an output expectation). + FAILURE_TO_CHAR = {test_expectations.CRASH: "C", + test_expectations.TIMEOUT: "T", + test_expectations.IMAGE: "I", + test_expectations.TEXT: "F", + test_expectations.MISSING: "O", + test_expectations.IMAGE_PLUS_TEXT: "Z"} + def __init__(self, port, builder_name, build_name, build_number, results_file_base_path, builder_base_url, test_timings, expectations, result_summary, all_tests): @@ -53,20 +63,14 @@ class JSONLayoutResultsGenerator(json_results_generator.JSONResultsGenerator): Args: result_summary: ResultsSummary object storing the summary of the test results. - (see the comment of JSONResultsGenerator.__init__ for other Args) """ + super(JSONLayoutResultsGenerator, self).__init__( + builder_name, build_name, build_number, results_file_base_path, + builder_base_url, {}, port.test_repository_paths()) + self._port = port - self._builder_name = builder_name - self._build_name = build_name - self._build_number = build_number - self._builder_base_url = builder_base_url - self._results_file_path = os.path.join(results_file_base_path, - self.RESULTS_FILENAME) self._expectations = expectations - # We don't use self._skipped_tests and self._passed_tests as we - # override _InsertFailureSummaries. - # We want relative paths to LayoutTest root for JSON output. path_to_name = self._get_path_relative_to_layout_test_root self._result_summary = result_summary @@ -77,9 +81,8 @@ class JSONLayoutResultsGenerator(json_results_generator.JSONResultsGenerator): self._test_timings = dict( (path_to_name(test_tuple.filename), test_tuple.test_run_time) for test_tuple in test_timings) - self._svn_repositories = port.test_repository_paths() - self._generate_json_output() + self.generate_json_output() def _get_path_relative_to_layout_test_root(self, test): """Returns the path of the test relative to the layout test root. @@ -102,6 +105,27 @@ class JSONLayoutResultsGenerator(json_results_generator.JSONResultsGenerator): return relativePath.replace('\\', '/') # override + def _get_test_timing(self, test_name): + if test_name in self._test_timings: + # Floor for now to get time in seconds. + return int(self._test_timings[test_name]) + return 0 + + # override + def _get_failed_test_names(self): + return set(self._failures.keys()) + + # override + def _get_result_type_char(self, test_name): + if test_name not in self._all_tests: + return self.NO_DATA_RESULT + + if test_name in self._failures: + return self.FAILURE_TO_CHAR[self._failures[test_name]] + + return self.PASS_RESULT + + # override def _convert_json_to_current_version(self, results_json): archive_version = None if self.VERSION_KEY in results_json: 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 1cf1b95..595fc2b 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 @@ -1,4 +1,3 @@ -#!/usr/bin/env python # Copyright (C) 2010 Google Inc. All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -38,16 +37,27 @@ import time import urllib2 import xml.dom.minidom -from webkitpy.common.checkout import scm -from webkitpy.common.system.executive import ScriptError -from webkitpy.layout_tests.layout_package import test_expectations import webkitpy.thirdparty.simplejson as simplejson -_log = logging.getLogger("webkitpy.layout_tests.layout_package." - "json_results_generator") +# A JSON results generator for generic tests. +# FIXME: move this code out of the layout_package directory. +_log = logging.getLogger("webkitpy.layout_tests.layout_package.json_results_generator") -class JSONResultsGenerator(object): + +class TestResult(object): + """A simple class that represents a single test result.""" + def __init__(self, name, failed=False, skipped=False, elapsed_time=0): + self.name = name + self.failed = failed + self.skipped = skipped + self.time = elapsed_time + + def fixable(self): + return self.failed or self.skipped + + +class JSONResultsGeneratorBase(object): """A JSON results generator for generic tests.""" MAX_NUMBER_OF_BUILD_RESULTS_TO_LOG = 750 @@ -57,6 +67,7 @@ class JSONResultsGenerator(object): JSON_SUFFIX = ");" PASS_RESULT = "P" SKIP_RESULT = "X" + FAIL_RESULT = "F" NO_DATA_RESULT = "N" VERSION = 3 VERSION_KEY = "version" @@ -70,22 +81,11 @@ class JSONResultsGenerator(object): FIXABLE = "fixableCounts" ALL_FIXABLE_COUNT = "allFixableCount" - # Note that we omit test_expectations.FAIL from this list because - # it should never show up (it's a legacy input expectation, never - # an output expectation). - FAILURE_TO_CHAR = {test_expectations.CRASH: "C", - test_expectations.TIMEOUT: "T", - test_expectations.IMAGE: "I", - test_expectations.TEXT: "F", - test_expectations.MISSING: "O", - test_expectations.IMAGE_PLUS_TEXT: "Z"} - FAILURE_CHARS = FAILURE_TO_CHAR.values() - RESULTS_FILENAME = "results.json" - def __init__(self, port, builder_name, build_name, build_number, + def __init__(self, builder_name, build_name, build_number, results_file_base_path, builder_base_url, - test_timings, failures, passed_tests, skipped_tests, all_tests): + test_results_map, svn_repositories=None): """Modifies the results.json file. Grabs it off the archive directory if it is not found locally. @@ -96,12 +96,11 @@ class JSONResultsGenerator(object): results_file_base_path: Absolute path to the directory containing the results json file. builder_base_url: the URL where we have the archived test results. - test_timings: Map of test name to a test_run-time. - failures: Map of test name to a failure type (of test_expectations). - passed_tests: A set containing all the passed tests. - skipped_tests: A set containing all the skipped tests. - all_tests: List of all the tests that were run. This should not - include skipped tests. + If this is None no archived results will be retrieved. + test_results_map: A dictionary that maps test_name to TestResult. + svn_repositories: A (json_field_name, svn_path) pair for SVN + repositories that tests rely on. The SVN revision will be + included in the JSON with the given json_field_name. """ self._builder_name = builder_name self._build_name = build_name @@ -109,31 +108,106 @@ class JSONResultsGenerator(object): self._builder_base_url = builder_base_url self._results_file_path = os.path.join(results_file_base_path, self.RESULTS_FILENAME) - self._test_timings = test_timings - self._failures = failures - self._passed_tests = passed_tests - self._skipped_tests = skipped_tests - self._all_tests = all_tests - self._svn_repositories = port.test_repository_paths() - self._generate_json_output() + self._test_results_map = test_results_map + self._test_results = test_results_map.values() + + self._svn_repositories = svn_repositories + if not self._svn_repositories: + self._svn_repositories = {} + + self._json = None - def _generate_json_output(self): + def generate_json_output(self): """Generates the JSON output file.""" - json = self._get_json() - if json: + if not self._json: + self._json = self.get_json() + if self._json: + # Specify separators in order to get compact encoding. + json_data = simplejson.dumps(self._json, separators=(',', ':')) + json_string = self.JSON_PREFIX + json_data + self.JSON_SUFFIX + results_file = codecs.open(self._results_file_path, "w", "utf-8") - results_file.write(json) + results_file.write(json_string) results_file.close() + def get_json(self): + """Gets the results for the results.json file.""" + if self._json: + return self._json + + results_json, error = self._get_archived_json_results() + if error: + # If there was an error don't write a results.json + # file at all as it would lose all the information on the bot. + _log.error("Archive directory is inaccessible. Not modifying " + "or clobbering the results.json file: " + str(error)) + return None + + builder_name = self._builder_name + if results_json and builder_name not in results_json: + _log.debug("Builder name (%s) is not in the results.json file." + % builder_name) + + self._convert_json_to_current_version(results_json) + + if builder_name not in results_json: + results_json[builder_name] = ( + self._create_results_for_builder_json()) + + results_for_builder = results_json[builder_name] + + self._insert_generic_metadata(results_for_builder) + + self._insert_failure_summaries(results_for_builder) + + # Update the all failing tests with result type and time. + tests = results_for_builder[self.TESTS] + all_failing_tests = self._get_failed_test_names() + all_failing_tests.update(tests.iterkeys()) + for test in all_failing_tests: + self._insert_test_time_and_result(test, tests) + + self._json = results_json + return self._json + + def _get_test_timing(self, test_name): + """Returns test timing data (elapsed time) in second + for the given test_name.""" + if test_name in self._test_results_map: + # Floor for now to get time in seconds. + return int(self._test_results_map[test_name].time) + return 0 + + def _get_failed_test_names(self): + """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): + """Returns a single char (e.g. SKIP_RESULT, FAIL_RESULT, + PASS_RESULT, NO_DATA_RESULT, etc) that indicates the test result + 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 + + return JSONResultsGenerator.PASS_RESULT + # FIXME: Callers should use scm.py instead. + # FIXME: Identify and fix the run-time errors that were observed on Windows + # chromium buildbot when we had updated this code to use scm.py once before. def _get_svn_revision(self, in_directory): """Returns the svn revision for the given directory. Args: in_directory: The directory where svn is to be run. """ - if os.path.exists(os.path.join(in_directory, '.svn')): # Note: Not thread safe: http://bugs.python.org/issue2320 output = subprocess.Popen(["svn", "info", "--xml"], @@ -196,76 +270,34 @@ class JSONResultsGenerator(object): return results_json, error - def _get_json(self): - """Gets the results for the results.json file.""" - results_json, error = self._get_archived_json_results() - if error: - # If there was an error don't write a results.json - # file at all as it would lose all the information on the bot. - _log.error("Archive directory is inaccessible. Not modifying " - "or clobbering the results.json file: " + str(error)) - return None - - builder_name = self._builder_name - if results_json and builder_name not in results_json: - _log.debug("Builder name (%s) is not in the results.json file." - % builder_name) - - self._convert_json_to_current_version(results_json) - - if builder_name not in results_json: - results_json[builder_name] = ( - self._create_results_for_builder_json()) - - results_for_builder = results_json[builder_name] - - self._insert_generic_metadata(results_for_builder) - - self._insert_failure_summaries(results_for_builder) - - # Update the all failing tests with result type and time. - tests = results_for_builder[self.TESTS] - all_failing_tests = set(self._failures.iterkeys()) - all_failing_tests.update(tests.iterkeys()) - for test in all_failing_tests: - self._insert_test_time_and_result(test, tests) - - # Specify separators in order to get compact encoding. - results_str = simplejson.dumps(results_json, separators=(',', ':')) - return self.JSON_PREFIX + results_str + self.JSON_SUFFIX - def _insert_failure_summaries(self, results_for_builder): """Inserts aggregate pass/failure statistics into the JSON. - This method reads self._skipped_tests, self._passed_tests and - self._failures and inserts FIXABLE, FIXABLE_COUNT and ALL_FIXABLE_COUNT - entries. + This method reads self._test_results and generates + FIXABLE, FIXABLE_COUNT and ALL_FIXABLE_COUNT entries. Args: results_for_builder: Dictionary containing the test results for a single builder. """ - # Insert the number of tests that failed. + # Insert the number of tests that failed or skipped. + fixable_count = len([r for r in self._test_results if r.fixable()]) self._insert_item_into_raw_list(results_for_builder, - len(set(self._failures.keys()) | self._skipped_tests), - self.FIXABLE_COUNT) + fixable_count, self.FIXABLE_COUNT) # Create a pass/skip/failure summary dictionary. entry = {} - entry[self.SKIP_RESULT] = len(self._skipped_tests) - entry[self.PASS_RESULT] = len(self._passed_tests) - get = entry.get - for failure_type in self._failures.values(): - failure_char = self.FAILURE_TO_CHAR[failure_type] - entry[failure_char] = get(failure_char, 0) + 1 + for test_name in self._test_results_map.iterkeys(): + result_char = self._get_result_type_char(test_name) + entry[result_char] = entry.get(result_char, 0) + 1 # Insert the pass/skip/failure summary dictionary. self._insert_item_into_raw_list(results_for_builder, entry, self.FIXABLE) # Insert the number of all the tests that are supposed to pass. + all_test_count = len(self._test_results) self._insert_item_into_raw_list(results_for_builder, - len(self._skipped_tests | self._all_tests), - self.ALL_FIXABLE_COUNT) + all_test_count, self.ALL_FIXABLE_COUNT) def _insert_item_into_raw_list(self, results_for_builder, item, key): """Inserts the item into the list with the given key in the results for @@ -331,18 +363,8 @@ class JSONResultsGenerator(object): tests: Dictionary containing test result entries. """ - result = JSONResultsGenerator.PASS_RESULT - time = 0 - - if test_name not in self._all_tests: - result = JSONResultsGenerator.NO_DATA_RESULT - - if test_name in self._failures: - result = self.FAILURE_TO_CHAR[self._failures[test_name]] - - if test_name in self._test_timings: - # Floor for now to get time in seconds. - time = int(self._test_timings[test_name]) + result = self._get_result_type_char(test_name) + time = self._get_test_timing(test_name) if test_name not in tests: tests[test_name] = self._create_results_and_times_json() @@ -420,3 +442,59 @@ class JSONResultsGenerator(object): """Returns whether all the results are of the given type (e.g. all passes).""" return len(results) == 1 and results[0][1] == type + + +# A wrapper class for JSONResultsGeneratorBase. +# Note: There's a script outside the WebKit codebase calling this script. +# FIXME: Please keep the interface until the other script is cleaned up. +# (http://src.chromium.org/viewvc/chrome/trunk/src/webkit/tools/layout_tests/webkitpy/layout_tests/test_output_xml_to_json.py?view=markup) +class JSONResultsGenerator(JSONResultsGeneratorBase): + # The flag is for backward compatibility. + output_json_in_init = True + + 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): + """Generates a JSON results file. + + Args + builder_name: the builder name (e.g. Webkit). + build_name: the build name (e.g. webkit-rel). + build_number: the build number. + results_file_base_path: Absolute path to the directory containing the + results json file. + builder_base_url: the URL where we have the archived test results. + test_timings: Map of test name to a test_run-time. + failures: Map of test name to a failure type (of test_expectations). + passed_tests: A set containing all the passed tests. + skipped_tests: A set containing all the skipped tests. + all_tests: List of all the tests that were run. This should not + include skipped tests. + """ + + # Create a map of (name, TestResult). + test_results_map = dict() + get = test_results_map.get + for (test, time) in test_timings.iteritems(): + test_results_map[test] = TestResult(test, elapsed_time=time) + for test in failures.iterkeys(): + test_results_map[test] = test_result = get(test, TestResult(test)) + 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) + + super(JSONResultsGenerator, self).__init__( + builder_name, build_name, build_number, + results_file_base_path, builder_base_url, test_results_map, + svn_repositories=port.test_repository_paths()) + + if self.__class__.output_json_in_init: + self.generate_json_output() 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 new file mode 100644 index 0000000..0a60cc7 --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator_unittest.py @@ -0,0 +1,126 @@ +# 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 json_results_generator.py.""" + +import unittest +import optparse +import random +import shutil +import tempfile + +from webkitpy.layout_tests.layout_package import json_results_generator +from webkitpy.layout_tests.layout_package import test_expectations +from webkitpy.layout_tests import port + + +class JSONGeneratorTest(unittest.TestCase): + def setUp(self): + json_results_generator.JSONResultsGenerator.output_json_in_init = False + self.builder_name = 'DUMMY_BUILDER_NAME' + self.build_name = 'DUMMY_BUILD_NAME' + self.build_number = 'DUMMY_BUILDER_NUMBER' + + 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()) + test_timings = {} + for test in tests_list: + test_timings[test] = float(random.randint(1, 10)) + + port_obj = port.get(None) + + # Generate a JSON file. + 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, + passed_tests, + skipped_tests, + tests_list) + + json = generator.get_json() + + # Aliasing to a short name for better access to its constants. + JRG = json_results_generator.JSONResultsGenerator + + self.assertTrue(JRG.VERSION_KEY in json) + self.assertTrue(self.builder_name in json) + + buildinfo = json[self.builder_name] + self.assertTrue(JRG.FIXABLE in buildinfo) + self.assertTrue(JRG.TESTS in buildinfo) + self.assertTrue(len(buildinfo[JRG.BUILD_NUMBERS]) == 1) + self.assertTrue(buildinfo[JRG.BUILD_NUMBERS][0] == self.build_number) + + if tests_list or skipped_tests: + fixable = buildinfo[JRG.FIXABLE][0] + if passed_tests: + self.assertTrue(fixable[JRG.PASS_RESULT] == len(passed_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)) + else: + self.assertTrue(JRG.SKIP_RESULT not in fixable or + fixable[JRG.SKIP_RESULT] == 0) + + if failed_tests: + tests = buildinfo[JRG.TESTS] + for test_name, failure in failed_tests.iteritems(): + self.assertTrue(test_name in tests) + test = tests[test_name] + self.assertTrue(test[JRG.RESULTS][0][0] == 1) + self.assertTrue(test[JRG.RESULTS][0][1] == JRG.FAIL_RESULT) + self.assertTrue(test[JRG.TIMES][0][0] == 1) + self.assertTrue(test[JRG.TIMES][0][1] == + int(test_timings[test_name])) + + fixable_count = len(skipped_tests) + len(failed_tests.keys()) + if skipped_tests or failed_tests: + self.assertTrue(buildinfo[JRG.FIXABLE_COUNT][0] == fixable_count) + + def test_json_generation(self): + reason = test_expectations.TEXT + + self._test_json_generation([], {}, []) + self._test_json_generation(['A', 'B'], {}, []) + self._test_json_generation([], {'A': reason, 'B': reason}, []) + self._test_json_generation([], {}, ['A', 'B']) + self._test_json_generation(['A'], {'B': reason, 'C': reason}, []) + self._test_json_generation([], {'A': reason, 'B': reason}, ['C', 'D']) + self._test_json_generation(['A', 'B', 'C'], {'D': reason}, ['E', 'F']) + + +if __name__ == '__main__': + unittest.main() diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py index e73579f..e1b23ac 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py @@ -128,13 +128,16 @@ class Port(object): return expected_text != actual_text def diff_image(self, expected_filename, actual_filename, - diff_filename=None): + diff_filename=None, tolerance=0): """Compare two image files and produce a delta image file. Return True if the two files are different, False if they are the same. Also produce a delta image of the two images and write that into |diff_filename| if it is not None. + |tolerance| should be a percentage value (0.0 - 100.0). + If it is omitted, the port default tolerance value is used. + While this is a generic routine, we include it in the Port interface so that it can be overriden for testing purposes.""" executable = self._path_to_image_diff() diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/qt.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/qt.py index 9032a24..41b2ba0 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/qt.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/qt.py @@ -32,6 +32,8 @@ import logging import os import signal +import webkit + from webkitpy.layout_tests.port.webkit import WebKitPort _log = logging.getLogger("webkitpy.layout_tests.port.qt") @@ -90,3 +92,8 @@ class QtPort(WebKitPort): def _path_to_driver(self): return self._build_path('bin/DumpRenderTree') + + def setup_environ_for_server(self): + env = webkit.WebKitPort.setup_environ_for_server(self) + env['QTWEBKIT_PLUGIN_PATH'] = self._build_path('lib/plugins') + return env diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py index e6d4c99..6eef54e 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py @@ -60,7 +60,7 @@ class TestPort(base.Port): return False def diff_image(self, expected_filename, actual_filename, - diff_filename=None): + diff_filename=None, tolerance=0): return False def diff_text(self, expected_text, actual_text, diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py index 2097ce7..e1151a6 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py @@ -116,10 +116,13 @@ class WebKitPort(base.Port): return True def diff_image(self, expected_filename, actual_filename, - diff_filename=None): + diff_filename=None, tolerance=0.1): """Return True if the two files are different. Also write a delta image of the two images into |diff_filename| if it is not None.""" + # FIXME: either expose the tolerance argument as a command-line + # parameter, or make it go away and always use exact matches. + # Handle the case where the test didn't actually generate an image. actual_length = os.stat(actual_filename).st_size if actual_length == 0: @@ -127,13 +130,11 @@ class WebKitPort(base.Port): shutil.copyfile(actual_filename, expected_filename) return True - sp = self._diff_image_request(expected_filename, actual_filename) + sp = self._diff_image_request(expected_filename, actual_filename, tolerance) return self._diff_image_reply(sp, expected_filename, diff_filename) - def _diff_image_request(self, expected_filename, actual_filename): - # FIXME: either expose the tolerance argument as a command-line - # parameter, or make it go away and aways use exact matches. - command = [self._path_to_image_diff(), '--tolerance', '0.1'] + def _diff_image_request(self, expected_filename, actual_filename, tolerance): + command = [self._path_to_image_diff(), '--tolerance', str(tolerance)] sp = server_process.ServerProcess(self, 'ImageDiff', command) actual_length = os.stat(actual_filename).st_size @@ -378,7 +379,7 @@ class WebKitDriver(base.Driver): command += [self._port._path_to_driver(), '-'] if self._image_path: command.append('--pixel-tests') - environment = os.environ + environment = self._port.setup_environ_for_server() environment['DYLD_FRAMEWORK_PATH'] = self._port._build_path() self._server_process = server_process.ServerProcess(self._port, "DumpRenderTree", command, environment) diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py b/WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py index 35f32d4..fa4df9b 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py @@ -916,8 +916,14 @@ def main(): option_parser.add_option('-d', '--html_directory', default='', - help=('The directory that stores the results for' - ' rebaselining comparison.')) + help=('The directory that stores the results for ' + 'rebaselining comparison.')) + + option_parser.add_option('', '--use_drt', + action='store_true', + default=False, + help=('Use ImageDiff from DumpRenderTree instead ' + 'of image_diff for pixel tests.')) option_parser.add_option('', '--target-platform', default='chromium', diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py b/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py index a4a92c7..41aab62 100755 --- a/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py @@ -1619,6 +1619,8 @@ def parse_args(args=None): help="run all tests in parallel"), # FIXME: Need --exit-after-n-failures N # Exit after the first N failures instead of running all tests + # FIXME: Need --exit-after-n-crashes N + # Exit after the first N crashes instead of running all tests # FIXME: consider: --iterations n # Number of times to run the set of tests (e.g. ABCABCABC) optparse.make_option("--print-last-failures", action="store_true", 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 fe73a7b..65f8f3a 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/test_types/image_diff.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/test_types/image_diff.py @@ -192,7 +192,7 @@ class ImageDiff(test_type_base.TestTypeBase): return failures def diff_files(self, port, file1, file2): - """Diff two image files. + """Diff two image files exactly. Args: file1, file2: full paths of the files to compare. @@ -202,6 +202,6 @@ class ImageDiff(test_type_base.TestTypeBase): False otherwise. """ try: - return port.diff_image(file1, file2) + return port.diff_image(file1, file2, None, 0) except ValueError, e: return True diff --git a/WebKitTools/Scripts/webkitpy/style/optparser.py b/WebKitTools/Scripts/webkitpy/style/optparser.py index bb4788a..3ba0fae 100644 --- a/WebKitTools/Scripts/webkitpy/style/optparser.py +++ b/WebKitTools/Scripts/webkitpy/style/optparser.py @@ -147,8 +147,7 @@ class CommandOptionValues(object): git_commit=None, is_verbose=False, min_confidence=1, - output_format="emacs", - squash=False): + output_format="emacs"): if filter_rules is None: filter_rules = [] @@ -167,7 +166,6 @@ class CommandOptionValues(object): self.is_verbose = is_verbose self.min_confidence = min_confidence self.output_format = output_format - self.squash = squash # Useful for unit testing. def __eq__(self, other): @@ -182,8 +180,6 @@ class CommandOptionValues(object): return False if self.output_format != other.output_format: return False - if self.squash != other.squash: - return False return True @@ -218,8 +214,6 @@ class ArgumentPrinter(object): flags['filter'] = ",".join(filter_rules) if options.git_commit: flags['git-commit'] = options.git_commit - if options.squash: - flags['squash'] = options.squash flag_string = '' # Alphabetizing lets us unit test this method. @@ -309,7 +303,7 @@ class ArgumentParser(object): parser.add_option("-f", "--filter-rules", metavar="RULES", dest="filter_value", help=filter_help) - git_commit_help = ("check all changes in the given git commit. " + git_commit_help = ("check all changes in the given commit. " "Use 'commit_id..' to check all changes after commmit_id") parser.add_option("-g", "--git-diff", "--git-commit", metavar="COMMIT", dest="git_commit", help=git_commit_help,) @@ -330,14 +324,6 @@ class ArgumentParser(object): dest="output_format", default=default_output_format, help=output_format_help) - squash_help = ("All diffs from the remote branch are checked." - "If excluded, prompts whether to squash when there are multiple commits.") - parser.add_option("-s", "--squash", action="store_true", dest="squash", help=squash_help) - - squash_help = ("Only working copy diffs are checked." - "If excluded, prompts whether to squash when there are multiple commits.") - parser.add_option("--no-squash", action="store_false", dest="squash", help=squash_help) - verbose_help = "enable verbose logging." parser.add_option("-v", "--verbose", dest="is_verbose", default=False, action="store_true", help=verbose_help) @@ -458,8 +444,7 @@ class ArgumentParser(object): git_commit=git_commit, is_verbose=is_verbose, min_confidence=min_confidence, - output_format=output_format, - squash=options.squash) + output_format=output_format) return (paths, options) diff --git a/WebKitTools/Scripts/webkitpy/style_references.py b/WebKitTools/Scripts/webkitpy/style_references.py index bab30ca..a42b69d 100644 --- a/WebKitTools/Scripts/webkitpy/style_references.py +++ b/WebKitTools/Scripts/webkitpy/style_references.py @@ -67,6 +67,6 @@ class WebKitCheckout(object): """Return the checkout root as an absolute path.""" return self._scm.checkout_root - def create_patch(self, git_commit, squash): - return self._scm.create_patch(git_commit, squash) + def create_patch(self, git_commit): + return self._scm.create_patch(git_commit) diff --git a/WebKitTools/Scripts/webkitpy/thirdparty/__init__.py b/WebKitTools/Scripts/webkitpy/thirdparty/__init__.py index e1fa673..3321293 100644 --- a/WebKitTools/Scripts/webkitpy/thirdparty/__init__.py +++ b/WebKitTools/Scripts/webkitpy/thirdparty/__init__.py @@ -42,6 +42,9 @@ from webkitpy.common.system.autoinstall import AutoInstaller # perhaps be done using Python's import hooks as the original # autoinstall implementation did. +# FIXME: If any of these servers is offline, webkit-patch breaks (and maybe +# other scripts do, too). See <http://webkit.org/b/42080>. + # We put auto-installed third-party modules in this directory-- # # webkitpy/thirdparty/autoinstalled @@ -85,9 +88,9 @@ installer.install(url="http://webkit-rietveld.googlecode.com/svn/trunk/static/up # organization purposes. irc_dir = os.path.join(autoinstalled_dir, "irc") installer = AutoInstaller(target_dir=irc_dir) -installer.install(url="http://hivelocity.dl.sourceforge.net/project/python-irclib/python-irclib/0.4.8/python-irclib-0.4.8.zip", +installer.install(url="http://surfnet.dl.sourceforge.net/project/python-irclib/python-irclib/0.4.8/python-irclib-0.4.8.zip", url_subpath="irclib.py") -installer.install(url="http://hivelocity.dl.sourceforge.net/project/python-irclib/python-irclib/0.4.8/python-irclib-0.4.8.zip", +installer.install(url="http://surfnet.dl.sourceforge.net/project/python-irclib/python-irclib/0.4.8/python-irclib-0.4.8.zip", url_subpath="ircbot.py") pywebsocket_dir = os.path.join(autoinstalled_dir, "pywebsocket") diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/commandtest.py b/WebKitTools/Scripts/webkitpy/tool/commands/commandtest.py index 887802c..de92cd3 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/commandtest.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/commandtest.py @@ -29,10 +29,9 @@ import unittest from webkitpy.common.system.outputcapture import OutputCapture -from webkitpy.tool.mocktool import MockTool -from webkitpy.thirdparty.mock import Mock +from webkitpy.tool.mocktool import MockOptions, MockTool class CommandsTest(unittest.TestCase): - def assert_execute_outputs(self, command, args, expected_stdout="", expected_stderr="", options=Mock(), tool=MockTool()): + def assert_execute_outputs(self, command, args, expected_stdout="", expected_stderr="", options=MockOptions(), tool=MockTool()): command.bind_to_tool(tool) OutputCapture().assert_outputs(self, command.execute, [options, args, tool], expected_stdout=expected_stdout, expected_stderr=expected_stderr) diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/download.py b/WebKitTools/Scripts/webkitpy/tool/commands/download.py index 59af16a..d27ab0e 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/download.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/download.py @@ -93,7 +93,7 @@ If a bug id is provided, or one can be found in the ChangeLog land will update t def _prepare_state(self, options, args, tool): return { - "bug_id": (args and args[0]) or tool.checkout().bug_id_for_this_commit(options.git_commit, options.squash), + "bug_id": (args and args[0]) or tool.checkout().bug_id_for_this_commit(options.git_commit), } diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py index 958620a..75cd0f3 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py @@ -32,7 +32,7 @@ from webkitpy.common.system.outputcapture import OutputCapture from webkitpy.thirdparty.mock import Mock from webkitpy.tool.commands.commandtest import CommandsTest from webkitpy.tool.commands.download import * -from webkitpy.tool.mocktool import MockTool +from webkitpy.tool.mocktool import MockOptions, MockTool class AbstractRolloutPrepCommandTest(unittest.TestCase): @@ -56,7 +56,7 @@ class AbstractRolloutPrepCommandTest(unittest.TestCase): class DownloadCommandsTest(CommandsTest): def _default_options(self): - options = Mock() + options = MockOptions() options.force_clean = False options.clean = True options.check_builders = True diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py index 5ff390c..97c3ddb 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py @@ -42,7 +42,7 @@ from webkitpy.tool.commands.stepsequence import StepSequenceErrorHandler from webkitpy.tool.bot.patchcollection import PersistentPatchCollection, PersistentPatchCollectionDelegate from webkitpy.tool.bot.queueengine import QueueEngine, QueueEngineDelegate from webkitpy.tool.grammar import pluralize -from webkitpy.tool.multicommandtool import Command +from webkitpy.tool.multicommandtool import Command, TryAgain class AbstractQueue(Command, QueueEngineDelegate): watchers = [ @@ -279,6 +279,17 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler): validator = CommitterValidator(tool.bugs) validator.reject_patch_from_commit_queue(state["patch"].id(), cls._error_message_for_bug(tool, status_id, script_error)) + @classmethod + def handle_checkout_needs_update(cls, tool, state, options, error): + # The only time when we find out that out checkout needs update is + # when we were ready to actually pull the trigger and land the patch. + # Rather than spinning in the master process, we retry without + # building or testing, which is much faster. + options.build = False + options.test = False + options.update = True + raise TryAgain() + class RietveldUploadQueue(AbstractPatchQueue, StepSequenceErrorHandler): name = "rietveld-upload-queue" diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/stepsequence.py b/WebKitTools/Scripts/webkitpy/tool/commands/stepsequence.py index c6de79f..be2ed4c 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/stepsequence.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/stepsequence.py @@ -39,6 +39,10 @@ class StepSequenceErrorHandler(): def handle_script_error(cls, tool, patch, script_error): raise NotImplementedError, "subclasses must implement" + @classmethod + def handle_checkout_needs_update(cls, tool, state, options, error): + raise NotImplementedError, "subclasses must implement" + class StepSequence(object): def __init__(self, steps): @@ -66,6 +70,9 @@ class StepSequence(object): self._run(tool, options, state) except CheckoutNeedsUpdate, e: log("Commit failed because the checkout is out of date. Please update and try again.") + if options.parent_command: + command = tool.command_by_name(options.parent_command) + command.handle_checkout_needs_update(tool, state, options, e) QueueEngine.exit_after_handled_error(e) except ScriptError, e: if not options.quiet: diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/upload.py b/WebKitTools/Scripts/webkitpy/tool/commands/upload.py index 9c935e8..4a15ed6 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/upload.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/upload.py @@ -53,7 +53,6 @@ class CommitMessageForCurrentDiff(AbstractDeclarativeCommand): def __init__(self): options = [ - steps.Options.squash, steps.Options.git_commit, ] AbstractDeclarativeCommand.__init__(self, options=options) @@ -61,7 +60,7 @@ class CommitMessageForCurrentDiff(AbstractDeclarativeCommand): def execute(self, options, args, tool): # This command is a useful test to make sure commit_message_for_this_commit # always returns the right value regardless of the current working directory. - print "%s" % tool.checkout().commit_message_for_this_commit(options.git_commit, options.squash).message() + print "%s" % tool.checkout().commit_message_for_this_commit(options.git_commit).message() class CleanPendingCommit(AbstractDeclarativeCommand): @@ -153,7 +152,7 @@ class AbstractPatchUploadingCommand(AbstractSequencedCommand): # Perfer a bug id passed as an argument over a bug url in the diff (i.e. ChangeLogs). bug_id = args and args[0] if not bug_id: - bug_id = tool.checkout().bug_id_for_this_commit(options.git_commit, options.squash) + bug_id = tool.checkout().bug_id_for_this_commit(options.git_commit) return bug_id def _prepare_state(self, options, args, tool): @@ -423,11 +422,11 @@ class CreateBug(AbstractDeclarativeCommand): if options.prompt: (bug_title, comment_text) = self.prompt_for_bug_title_and_comment() else: - commit_message = tool.checkout().commit_message_for_this_commit(options.git_commit, options.squash) + commit_message = tool.checkout().commit_message_for_this_commit(options.git_commit) bug_title = commit_message.description(lstrip=True, strip_url=True) comment_text = commit_message.body(lstrip=True) - diff = tool.scm().create_patch(options.git_commit, options.squash) + diff = tool.scm().create_patch(options.git_commit) bug_id = tool.bugs.create_bug(bug_title, comment_text, options.component, diff, "Patch", cc=options.cc, mark_for_review=options.review, mark_for_commit_queue=options.request_commit) def prompt_for_bug_title_and_comment(self): diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py index 8fef54a..5f3f400 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py @@ -29,7 +29,7 @@ from webkitpy.thirdparty.mock import Mock from webkitpy.tool.commands.commandtest import CommandsTest from webkitpy.tool.commands.upload import * -from webkitpy.tool.mocktool import MockTool +from webkitpy.tool.mocktool import MockOptions, MockTool class UploadCommandsTest(CommandsTest): def test_commit_message_for_current_diff(self): @@ -51,7 +51,7 @@ class UploadCommandsTest(CommandsTest): self.assert_execute_outputs(ObsoleteAttachments(), [42], expected_stderr=expected_stderr) def test_post(self): - options = Mock() + options = MockOptions() options.description = "MOCK description" options.request_commit = False options.review = True @@ -80,7 +80,7 @@ MOCK: user.open_url: http://example.com/42 self.assert_execute_outputs(Prepare(), [], expected_stderr=expected_stderr) def test_upload(self): - options = Mock() + options = MockOptions() options.description = "MOCK description" options.request_commit = False options.review = True diff --git a/WebKitTools/Scripts/webkitpy/tool/mocktool.py b/WebKitTools/Scripts/webkitpy/tool/mocktool.py index d88190f..a467364 100644 --- a/WebKitTools/Scripts/webkitpy/tool/mocktool.py +++ b/WebKitTools/Scripts/webkitpy/tool/mocktool.py @@ -397,7 +397,7 @@ class MockSCM(Mock): # will actually be the root. Since getcwd() is wrong, use a globally fake root for now. self.checkout_root = self.fake_checkout_root - def create_patch(self, git_commit, squash): + def create_patch(self, git_commit): return "Patch1" def commit_ids_from_commitish_arguments(self, args): @@ -437,12 +437,12 @@ class MockCheckout(object): def bug_id_for_revision(self, svn_revision): return 12345 - def modified_changelogs(self, git_commit, squash): + def modified_changelogs(self, git_commit): # Ideally we'd return something more interesting here. The problem is # that LandDiff will try to actually read the patch from disk! return [] - def commit_message_for_this_commit(self, git_commit, squash): + def commit_message_for_this_commit(self, git_commit): commit_message = Mock() commit_message.message = lambda:"This is a fake commit message that is at least 50 characters." return commit_message @@ -515,6 +515,7 @@ class MockStatusServer(object): def results_url_for_status(self, status_id): return "http://dummy_url" + class MockExecute(Mock): def __init__(self, should_log): self._should_log = should_log @@ -537,6 +538,11 @@ class MockExecute(Mock): return "MOCK output of child process" +class MockOptions(Mock): + no_squash = False + squash = False + + class MockRietveld(): def __init__(self, executive, dryrun=False): diff --git a/WebKitTools/Scripts/webkitpy/tool/multicommandtool.py b/WebKitTools/Scripts/webkitpy/tool/multicommandtool.py index 7940c06..12ede2e 100644 --- a/WebKitTools/Scripts/webkitpy/tool/multicommandtool.py +++ b/WebKitTools/Scripts/webkitpy/tool/multicommandtool.py @@ -39,6 +39,10 @@ from webkitpy.tool.grammar import pluralize from webkitpy.common.system.deprecated_logging import log +class TryAgain(Exception): + pass + + class Command(object): name = None show_in_main_help = False @@ -299,6 +303,12 @@ class MultiCommandTool(object): log(failure_reason) return 0 # FIXME: Should this really be 0? - result = command.check_arguments_and_execute(options, args, self) + while True: + try: + result = command.check_arguments_and_execute(options, args, self) + break + except TryAgain, e: + pass + self.command_completed() return result diff --git a/WebKitTools/Scripts/webkitpy/tool/multicommandtool_unittest.py b/WebKitTools/Scripts/webkitpy/tool/multicommandtool_unittest.py index 268ebf0..c19095c 100644 --- a/WebKitTools/Scripts/webkitpy/tool/multicommandtool_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/multicommandtool_unittest.py @@ -32,7 +32,7 @@ import unittest from optparse import make_option from webkitpy.common.system.outputcapture import OutputCapture -from webkitpy.tool.multicommandtool import MultiCommandTool, Command +from webkitpy.tool.multicommandtool import MultiCommandTool, Command, TryAgain class TrivialCommand(Command): @@ -44,10 +44,26 @@ class TrivialCommand(Command): def execute(self, options, args, tool): pass + class UncommonCommand(TrivialCommand): name = "uncommon" show_in_main_help = False + +class LikesToRetry(Command): + name = "likes-to-retry" + show_in_main_help = True + + def __init__(self, **kwargs): + Command.__init__(self, "help text", **kwargs) + self.execute_count = 0 + + def execute(self, options, args, tool): + self.execute_count += 1 + if self.execute_count < 2: + raise TryAgain() + + class CommandTest(unittest.TestCase): def test_name_with_arguments(self): command_with_args = TrivialCommand(argument_names="ARG1 ARG2") @@ -109,6 +125,12 @@ class MultiCommandToolTest(unittest.TestCase): exit_code = OutputCapture().assert_outputs(self, tool.main, [main_args], expected_stdout=expected_stdout, expected_stderr=expected_stderr) self.assertEqual(exit_code, expected_exit_code) + def test_retry(self): + likes_to_retry = LikesToRetry() + tool = TrivialTool(commands=[likes_to_retry]) + tool.main(["tool", "likes-to-retry"]) + self.assertEqual(likes_to_retry.execute_count, 2) + def test_global_help(self): tool = TrivialTool(commands=[TrivialCommand(), UncommonCommand()]) expected_common_commands_help = """Usage: trivial-tool [options] COMMAND [ARGS] diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/abstractstep.py b/WebKitTools/Scripts/webkitpy/tool/steps/abstractstep.py index 20f8bbf..8f0d153 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/abstractstep.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/abstractstep.py @@ -27,6 +27,7 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. from webkitpy.common.system.deprecated_logging import log +from webkitpy.common.system.executive import ScriptError from webkitpy.common.config.ports import WebKitPort from webkitpy.tool.steps.options import Options @@ -34,6 +35,10 @@ from webkitpy.tool.steps.options import Options class AbstractStep(object): def __init__(self, tool, options): self._tool = tool + if options.no_squash: + raise ScriptError('--no-squash has been removed. Use "--git-commit=HEAD.." or "-g HEAD.." to operate on the working copy.') + if options.squash: + raise ScriptError('--squash has been removed. It is now the default behavior if --git-commit is omitted.') self._options = options self._port = None @@ -53,8 +58,8 @@ class AbstractStep(object): return self._port _well_known_keys = { - "diff": lambda self, state: self._tool.scm().create_patch(self._options.git_commit, self._options.squash), - "changelogs": lambda self, state: self._tool.checkout().modified_changelogs(self._options.git_commit, self._options.squash), + "diff": lambda self, state: self._tool.scm().create_patch(self._options.git_commit), + "changelogs": lambda self, state: self._tool.checkout().modified_changelogs(self._options.git_commit), "bug_title": lambda self, state: self._tool.bugs.fetch_bug(state["bug_id"]).title(), } @@ -69,8 +74,9 @@ class AbstractStep(object): @classmethod def options(cls): return [ - # We need these options here because cached_lookup uses them. :( + # We need this option here because cached_lookup uses it. :( Options.git_commit, + # FIXME: Get rid of these. Options.no_squash, Options.squash, ] diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/applypatchwithlocalcommit.py b/WebKitTools/Scripts/webkitpy/tool/steps/applypatchwithlocalcommit.py index d6b026d..3dcd8d9 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/applypatchwithlocalcommit.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/applypatchwithlocalcommit.py @@ -39,5 +39,5 @@ class ApplyPatchWithLocalCommit(ApplyPatch): def run(self, state): ApplyPatch.run(self, state) if self._options.local_commit: - commit_message = self._tool.checkout().commit_message_for_this_commit(git_commit=None, squash=False) + commit_message = self._tool.checkout().commit_message_for_this_commit(git_commit=None) self._tool.scm().commit_locally_with_message(commit_message.message() or state["patch"].name()) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle.py b/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle.py index 93e6215..af38214 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle.py @@ -40,8 +40,6 @@ class CheckStyle(AbstractStep): Options.non_interactive, Options.check_style, Options.git_commit, - Options.no_squash, - Options.squash, ] def run(self, state): @@ -53,10 +51,6 @@ class CheckStyle(AbstractStep): if self._options.git_commit: args.append("--git-commit") args.append(self._options.git_commit) - if self._tool.scm().should_squash(self._options.squash): - args.append("--squash") - else: - args.append("--no-squash") try: self._run_script("check-webkit-style", args) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle_unittest.py b/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle_unittest.py deleted file mode 100644 index a23ea1b..0000000 --- a/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle_unittest.py +++ /dev/null @@ -1,46 +0,0 @@ -# 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 webkitpy.common.system.executive import ScriptError -from webkitpy.thirdparty.mock import Mock -from webkitpy.tool.mocktool import MockTool -from webkitpy.tool.steps.checkstyle import CheckStyle - - -class CheckStyleTest(unittest.TestCase): - def test_should_squash_error(self): - """should_squash can throw an error. That error should not be eaten by CheckStyle.""" - def should_squash(squash): - raise ScriptError(message="Dummy error") - - tool = MockTool() - tool._scm.should_squash = should_squash - step = CheckStyle(tool, Mock()) - self.assertRaises(ScriptError, step.run, []) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/closebugforlanddiff_unittest.py b/WebKitTools/Scripts/webkitpy/tool/steps/closebugforlanddiff_unittest.py index 4e7f9e6..0a56564 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/closebugforlanddiff_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/closebugforlanddiff_unittest.py @@ -29,13 +29,12 @@ import unittest from webkitpy.common.system.outputcapture import OutputCapture -from webkitpy.thirdparty.mock import Mock -from webkitpy.tool.mocktool import MockTool +from webkitpy.tool.mocktool import MockOptions, MockTool from webkitpy.tool.steps.closebugforlanddiff import CloseBugForLandDiff class CloseBugForLandDiffTest(unittest.TestCase): def test_empty_state(self): capture = OutputCapture() - step = CloseBugForLandDiff(MockTool(), Mock()) + step = CloseBugForLandDiff(MockTool(), MockOptions()) expected_stderr = "Committed r49824: <http://trac.webkit.org/changeset/49824>\nNo bug id provided.\n" capture.assert_outputs(self, step.run, [{"commit_text" : "Mock commit text"}], expected_stderr=expected_stderr) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/commit.py b/WebKitTools/Scripts/webkitpy/tool/steps/commit.py index 7bf8b8a..9f93120 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/commit.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/commit.py @@ -26,6 +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.scm import AuthenticationError, AmbiguousCommitError +from webkitpy.common.system.executive import ScriptError +from webkitpy.common.system.user import User from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options @@ -35,13 +38,38 @@ class Commit(AbstractStep): def options(cls): return AbstractStep.options() + [ Options.git_commit, - Options.no_squash, - Options.squash, ] + def _commit_warning(self, error): + working_directory_message = "" if error.working_directory_is_clean else " and working copy changes" + return ('There are %s local commits%s. Everything will be committed as a single commit. ' + 'To avoid this prompt, set "git config webkit-patch.squash true".' % ( + error.num_local_commits, working_directory_message)) + def run(self, state): - commit_message = self._tool.checkout().commit_message_for_this_commit(self._options.git_commit, self._options.squash) - if len(commit_message.message()) < 50: + self._commit_message = self._tool.checkout().commit_message_for_this_commit(self._options.git_commit).message() + if len(self._commit_message) < 50: raise Exception("Attempted to commit with a commit message shorter than 50 characters. Either your patch is missing a ChangeLog or webkit-patch may have a bug.") - state["commit_text"] = self._tool.scm().commit_with_message(commit_message.message(), - git_commit=self._options.git_commit, squash=self._options.squash) + + self._state = state + + username = None + force_squash = False + + num_tries = 0 + while num_tries < 3: + 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) + break; + except AmbiguousCommitError, e: + if self._tool.user.confirm(self._commit_warning(e)): + force_squash = True + else: + # This will correctly interrupt the rest of the commit process. + raise ScriptError(message="Did not commit") + except AuthenticationError, e: + username = self._tool.user.prompt("%s login: " % e.server_host, repeat=5) + if not username: + raise ScriptError("You need to specify the username on %s to perform the commit as." % self.svn_server_host) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/createbug.py b/WebKitTools/Scripts/webkitpy/tool/steps/createbug.py index cd043d6..0ab6f68 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/createbug.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/createbug.py @@ -36,6 +36,7 @@ class CreateBug(AbstractStep): return AbstractStep.options() + [ Options.cc, Options.component, + Options.blocks, ] def run(self, state): @@ -45,4 +46,7 @@ class CreateBug(AbstractStep): cc = self._options.cc if not cc: cc = state.get("bug_cc") - state["bug_id"] = self._tool.bugs.create_bug(state["bug_title"], state["bug_description"], blocked=state.get("bug_blocked"), component=self._options.component, cc=cc) + blocks = self._options.blocks + if not blocks: + blocks = state.get("bug_blocked") + state["bug_id"] = self._tool.bugs.create_bug(state["bug_title"], state["bug_description"], blocked=blocks, component=self._options.component, cc=cc) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/options.py b/WebKitTools/Scripts/webkitpy/tool/steps/options.py index fa36f73..9c73f5a 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/options.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/options.py @@ -29,6 +29,7 @@ from optparse import make_option class Options(object): + blocks = make_option("--blocks", action="store", dest="blocks", default=None, help="Bug number which the created bug blocks.") build = make_option("--build", action="store_true", dest="build", default=False, help="Build and run run-webkit-tests before committing.") build_style = make_option("--build-style", action="store", dest="build_style", default=None, help="Whether to build debug, release, or both.") cc = make_option("--cc", action="store", type="string", dest="cc", help="Comma-separated list of email addresses to carbon-copy.") @@ -42,11 +43,11 @@ class Options(object): description = make_option("-m", "--description", action="store", type="string", dest="description", help="Description string for the attachment (default: \"patch\")") email = make_option("--email", action="store", type="string", dest="email", help="Email address to use in ChangeLogs.") force_clean = make_option("--force-clean", action="store_true", dest="force_clean", default=False, help="Clean working directory before applying patches (removes local changes and commits)") -# FIXME: Make commit ranges treat each commit separately instead of squashing them into one. - git_commit = make_option("--git-commit", action="store", dest="git_commit", help="Local git commit to upload/land. If a range, the commits are squashed into one.") + git_commit = make_option("-g", "--git-commit", action="store", dest="git_commit", help="Operate on a local commit. If a range, the commits are squashed into one. HEAD.. operates on working copy changes only.") local_commit = make_option("--local-commit", action="store_true", dest="local_commit", default=False, help="Make a local commit for each applied patch") - no_squash = make_option("--no-squash", action="store_false", dest="squash", help="Don't squash local commits into one on upload/land (git-only).") non_interactive = make_option("--non-interactive", action="store_true", dest="non_interactive", default=False, help="Never prompt the user, fail as fast as possible.") + # FIXME: Remove --no-squash, once people have adjusted to using --git-commit. + no_squash = make_option("--no-squash", action="store_true", dest="no_squash", default=False, help="Obsolete. Use --git-commit=HEAD.. instead.") obsolete_patches = make_option("--no-obsolete", action="store_false", dest="obsolete_patches", default=True, help="Do not obsolete old patches before posting this one.") open_bug = make_option("--open-bug", action="store_true", dest="open_bug", default=False, help="Opens the associated bug in a browser.") parent_command = make_option("--parent-command", action="store", dest="parent_command", default=None, help="(Internal) The command that spawned this instance.") @@ -55,6 +56,7 @@ class Options(object): request_commit = make_option("--request-commit", action="store_true", dest="request_commit", default=False, help="Mark the patch as needing auto-commit after review.") review = make_option("--no-review", action="store_false", dest="review", default=True, help="Do not mark the patch for review.") reviewer = make_option("-r", "--reviewer", action="store", type="string", dest="reviewer", help="Update ChangeLogs to say Reviewed by REVIEWER.") - squash = make_option("-s", "--squash", action="store_true", dest="squash", help="Squash all local commits into one on upload/land (git-only).") + # FIXME: Remove --squash, once people have adjusted to using --git-commit. + squash = make_option("-s", "--squash", action="store_true", dest="squash", default=False, help="Obsolete. This is now the default behavior.") test = make_option("--test", action="store_true", dest="test", default=False, help="Run run-webkit-tests before committing.") update = make_option("--no-update", action="store_false", dest="update", default=True, help="Don't update the working directory.") diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog.py b/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog.py index 7f0c1a8..ce04024 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog.py @@ -43,8 +43,6 @@ class PrepareChangeLog(AbstractStep): Options.quiet, Options.email, Options.git_commit, - Options.no_squash, - Options.squash, ] def _ensure_bug_url(self, state): @@ -69,10 +67,9 @@ class PrepareChangeLog(AbstractStep): args.append("--bug=%s" % state["bug_id"]) if self._options.email: args.append("--email=%s" % self._options.email) - if self._tool.scm().should_squash(self._options.squash): - args.append("--merge-base=%s" % self._tool.scm().remote_merge_base()) - if self._options.git_commit: - args.append("--git-commit=%s" % self._options.git_commit) + + if self._tool.scm().supports_local_commits(): + args.append("--merge-base=%s" % self._tool.scm().merge_base(self._options.git_commit)) try: self._tool.executive.run_and_throw_if_fail(args, self._options.quiet) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog_unittest.py b/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog_unittest.py index 1d0db75..eceffdf 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog_unittest.py @@ -31,15 +31,14 @@ import unittest from webkitpy.common.checkout.changelog_unittest import ChangeLogTest from webkitpy.common.system.outputcapture import OutputCapture -from webkitpy.thirdparty.mock import Mock -from webkitpy.tool.mocktool import MockTool +from webkitpy.tool.mocktool import MockOptions, MockTool from webkitpy.tool.steps.preparechangelog import PrepareChangeLog class PrepareChangeLogTest(ChangeLogTest): def test_ensure_bug_url(self): capture = OutputCapture() - step = PrepareChangeLog(MockTool(), Mock()) + step = PrepareChangeLog(MockTool(), MockOptions()) changelog_contents = u"%s\n%s" % (self._new_entry_boilerplate, self._example_changelog) changelog_path = self._write_tmp_file_with_contents(changelog_contents.encode("utf-8")) state = { diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelogforrevert.py b/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelogforrevert.py index 4d299fa..0e78bc2 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelogforrevert.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelogforrevert.py @@ -36,7 +36,7 @@ class PrepareChangeLogForRevert(AbstractStep): def run(self, state): # This could move to prepare-ChangeLog by adding a --revert= option. self._run_script("prepare-ChangeLog") - changelog_paths = self._tool.checkout().modified_changelogs(git_commit=None, squash=False) + changelog_paths = self._tool.checkout().modified_changelogs(git_commit=None) bug_url = self._tool.bugs.bug_url_for_bug_id(state["bug_id"]) if state["bug_id"] else None for changelog_path in changelog_paths: # FIXME: Seems we should prepare the message outside of changelogs.py and then just pass in diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py b/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py index 1fd2bad..766801b 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py @@ -30,8 +30,7 @@ import unittest from webkitpy.common.system.outputcapture import OutputCapture from webkitpy.common.config.ports import WebKitPort -from webkitpy.thirdparty.mock import Mock -from webkitpy.tool.mocktool import MockTool +from webkitpy.tool.mocktool import MockOptions, MockTool from webkitpy.tool.steps.update import Update from webkitpy.tool.steps.runtests import RunTests from webkitpy.tool.steps.promptforbugortitle import PromptForBugOrTitle @@ -42,13 +41,13 @@ class StepsTest(unittest.TestCase): if not tool: tool = MockTool() if not options: - options = Mock() + options = MockOptions() if not state: state = {} step(tool, options).run(state) def test_update_step(self): - options = Mock() + options = MockOptions() options.update = True expected_stderr = "Updating working directory\n" OutputCapture().assert_outputs(self, self._run_step, [Update, options], expected_stderr=expected_stderr) @@ -63,7 +62,7 @@ class StepsTest(unittest.TestCase): OutputCapture().assert_outputs(self, self._run_step, [RunTests], expected_stderr=expected_stderr) def test_runtests_leopard_commit_queue_hack(self): - mock_options = Mock() + mock_options = MockOptions() mock_options.non_interactive = True step = RunTests(MockTool(log_executive=True), mock_options) # FIXME: We shouldn't use a real port-object here, but there is too much to mock at the moment. diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/updatechangelogswithreview_unittest.py b/WebKitTools/Scripts/webkitpy/tool/steps/updatechangelogswithreview_unittest.py index 0534718..a037422 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/updatechangelogswithreview_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/updatechangelogswithreview_unittest.py @@ -29,18 +29,17 @@ import unittest from webkitpy.common.system.outputcapture import OutputCapture -from webkitpy.thirdparty.mock import Mock -from webkitpy.tool.mocktool import MockTool +from webkitpy.tool.mocktool import MockOptions, MockTool from webkitpy.tool.steps.updatechangelogswithreviewer import UpdateChangeLogsWithReviewer class UpdateChangeLogsWithReviewerTest(unittest.TestCase): def test_guess_reviewer_from_bug(self): capture = OutputCapture() - step = UpdateChangeLogsWithReviewer(MockTool(), Mock()) + step = UpdateChangeLogsWithReviewer(MockTool(), MockOptions()) expected_stderr = "0 reviewed patches on bug 75, cannot infer reviewer.\n" capture.assert_outputs(self, step._guess_reviewer_from_bug, [75], expected_stderr=expected_stderr) def test_empty_state(self): capture = OutputCapture() - step = UpdateChangeLogsWithReviewer(MockTool(), Mock()) + step = UpdateChangeLogsWithReviewer(MockTool(), MockOptions()) capture.assert_outputs(self, step.run, [{}]) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py b/WebKitTools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py index ef4baa2..e46b790 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py @@ -40,8 +40,6 @@ class UpdateChangeLogsWithReviewer(AbstractStep): return AbstractStep.options() + [ Options.git_commit, Options.reviewer, - Options.no_squash, - Options.squash, ] def _guess_reviewer_from_bug(self, bug_id): diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer.py b/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer.py index 9f4d44e..bdf729e 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer.py @@ -41,8 +41,6 @@ class ValidateReviewer(AbstractStep): def options(cls): return AbstractStep.options() + [ Options.git_commit, - Options.no_squash, - Options.squash, ] # FIXME: This should probably move onto ChangeLogEntry diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer_unittest.py b/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer_unittest.py index 9105102..d9b856a 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer_unittest.py @@ -30,8 +30,7 @@ import unittest from webkitpy.common.checkout.changelog import ChangeLogEntry from webkitpy.common.system.outputcapture import OutputCapture -from webkitpy.thirdparty.mock import Mock -from webkitpy.tool.mocktool import MockTool +from webkitpy.tool.mocktool import MockOptions, MockTool from webkitpy.tool.steps.validatereviewer import ValidateReviewer class ValidateReviewerTest(unittest.TestCase): @@ -48,7 +47,7 @@ class ValidateReviewerTest(unittest.TestCase): self.assertEqual(step._has_valid_reviewer(entry), expected) def test_has_valid_reviewer(self): - step = ValidateReviewer(MockTool(), Mock()) + step = ValidateReviewer(MockTool(), MockOptions()) self._test_review_text(step, "Reviewed by Eric Seidel.", True) self._test_review_text(step, "Reviewed by Eric Seidel", True) # Not picky about the '.' self._test_review_text(step, "Reviewed by Eric.", False) |