diff options
| author | Ben Murdoch <benm@google.com> | 2011-05-13 16:23:25 +0100 | 
|---|---|---|
| committer | Ben Murdoch <benm@google.com> | 2011-05-16 11:35:02 +0100 | 
| commit | 65f03d4f644ce73618e5f4f50dd694b26f55ae12 (patch) | |
| tree | f478babb801e720de7bfaee23443ffe029f58731 /Tools/Scripts | |
| parent | 47de4a2fb7262c7ebdb9cd133ad2c54c187454d0 (diff) | |
| download | external_webkit-65f03d4f644ce73618e5f4f50dd694b26f55ae12.zip external_webkit-65f03d4f644ce73618e5f4f50dd694b26f55ae12.tar.gz external_webkit-65f03d4f644ce73618e5f4f50dd694b26f55ae12.tar.bz2  | |
Merge WebKit at r75993: Initial merge by git.
Change-Id: I602bbdc3974787a3b0450456a30a7868286921c3
Diffstat (limited to 'Tools/Scripts')
45 files changed, 875 insertions, 221 deletions
diff --git a/Tools/Scripts/VCSUtils.pm b/Tools/Scripts/VCSUtils.pm index e61774c..70e9acd 100644 --- a/Tools/Scripts/VCSUtils.pm +++ b/Tools/Scripts/VCSUtils.pm @@ -398,7 +398,10 @@ sub adjustPathForRecentRenamings($)  {      my ($fullPath) = @_; -    if ($fullPath =~ m|^WebCore/| || $fullPath =~ m|^JavaScriptCore/|) { +    if ($fullPath =~ m|^WebCore/| +        || $fullPath =~ m|^JavaScriptCore/| +        || $fullPath =~ m|^WebKit/| +        || $fullPath =~ m|^WebKit2/|) {          return "Source/$fullPath";      }      return $fullPath; diff --git a/Tools/Scripts/build-webkit b/Tools/Scripts/build-webkit index c300da1..0b58113 100755 --- a/Tools/Scripts/build-webkit +++ b/Tools/Scripts/build-webkit @@ -126,7 +126,7 @@ my @features = (        define => "ENABLE_ACCELERATED_2D_CANVAS", default => 0, value => \$accelerated2dCanvasSupport },      { option => "blob", desc => "Toggle Blob support", -      define => "ENABLE_BLOB", default => (isAppleMacWebKit()), value => \$blobSupport }, +      define => "ENABLE_BLOB", default => (isAppleMacWebKit() || isGtk()), value => \$blobSupport },      { option => "channel-messaging", desc => "Toggle MessageChannel and MessagePort support",        define => "ENABLE_CHANNEL_MESSAGING", default => 1, value => \$channelMessagingSupport }, @@ -198,7 +198,7 @@ my @features = (        define => "ENABLE_METER_TAG", default => !isGtk() && !isAppleWinWebKit(), value => \$meterTagSupport },      { option => "netscape-plugin", desc => "Netscape Plugin support", -      define => "ENABLE_NETSCAPE_PLUGIN_API", default => !isWinCE(), value => \$netscapePluginSupport }, +      define => "ENABLE_NETSCAPE_PLUGIN_API", default => !isEfl(), value => \$netscapePluginSupport },      { option => "notifications", desc => "Toggle Desktop Notifications Support",        define => "ENABLE_NOTIFICATIONS", default => 0, value => \$notificationsSupport }, @@ -377,7 +377,7 @@ sub unlinkZeroFiles ()  }  # Check that all the project directories are there. -my @projects = ("Source/JavaScriptCore", "Source/WebCore", "WebKit"); +my @projects = ("Source/JavaScriptCore", "Source/WebCore", "Source/WebKit");  my @otherDirs = ("WebKitLibraries");  for my $dir (@projects, @otherDirs) { @@ -422,7 +422,7 @@ if (isGtk()) {      splice @projects, 0, 0, "Source/ThirdParty/ANGLE";      # WebKit2 is only supported in SnowLeopard and later at present. -    push @projects, ("WebKit2", "Tools/MiniBrowser") if osXVersion()->{"minor"} >= 6 and !$noWebKit2; +    push @projects, ("Source/WebKit2", "Tools/MiniBrowser") if osXVersion()->{"minor"} >= 6 and !$noWebKit2;      # Copy library and header from WebKitLibraries to a findable place in the product directory.      my @librariesToCopy = ( @@ -542,7 +542,7 @@ for my $dir (@projects) {      my $result = 0;      # For Gtk and Qt the WebKit project builds all others -    if ((isGtk() || isQt()) && $dir ne "WebKit") { +    if ((isGtk() || isQt()) && $dir ne "Source/WebKit") {          chdirWebKit();          next;      } diff --git a/Tools/Scripts/check-inspector-strings b/Tools/Scripts/check-inspector-strings new file mode 100755 index 0000000..82c08d7 --- /dev/null +++ b/Tools/Scripts/check-inspector-strings @@ -0,0 +1,115 @@ +#!/usr/bin/env python +# +# Copyright (C) 2011 Google Inc. All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +#    * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +#    * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +#    * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +import codecs +import logging +import re +import sys + +from webkitpy.style_references import detect_checkout +from webkitpy.common.system.logutils import configure_logging +from webkitpy.style.checker import ProcessorBase +from webkitpy.style.filereader import TextFileReader +from webkitpy.style.main import change_directory + +_inspector_directory = "Source/WebCore/inspector/front-end" +_localized_strings = "Source/WebCore/English.lproj/localizedStrings.js" + +_log = logging.getLogger("check-inspector-strings") + +class StringsExtractor(ProcessorBase): +    def __init__(self, patterns): +        self._patterns = patterns +        self.strings = [] +        for p in self._patterns: +            self.strings.append([]) + +    def should_process(self, file_path): +        return file_path.endswith(".js") and (not file_path.endswith("InjectedScript.js")) + +    def process(self, lines, file_path, line_numbers=None): +        for line in lines: +            comment_start = line.find("//") +            if comment_start != -1: +                line = line[:comment_start] +            index = 0 +            for pattern in self._patterns: +                line_strings = re.findall(pattern, line) +                for string in line_strings: +                    self.strings[index].append(string) +                index += 1 + +class LocalizedStringsExtractor: +    def __init__(self): +        self.localized_strings = [] + +    def process_file(self, file_path): +        localized_strings_file = codecs.open(file_path, encoding="utf-16", mode="r") +        try: +            contents = localized_strings_file.read() +            lines = contents.split("\n") +            for line in lines: +                match = re.match(r"localizedStrings\[\"((?:[^\"\\]|\\.)*?)\"", line) +                if match: +                    self.localized_strings.append(match.group(1)) +        finally: +            localized_strings_file.close() + +if __name__ == "__main__": +    configure_logging() + +    checkout = detect_checkout() +    if checkout is None: +        _log.error("WebKit checkout not found: You must run this script " +                   "from within a WebKit checkout.") +        sys.exit(1) +    checkout_root = checkout.root_path() +    _log.debug("WebKit checkout found with root: %s" % checkout_root) +    change_directory(checkout_root=checkout_root, paths=None) + +    strings_extractor = StringsExtractor([r"WebInspector\.(?:UIString|formatLocalized)\(\"((?:[^\"\\]|\\.)*?)\"", r"\"((?:[^\"\\]|\\.)*?)\""]) +    file_reader = TextFileReader(strings_extractor) +    file_reader.process_paths([_inspector_directory]) +    localized_strings_extractor = LocalizedStringsExtractor() +    localized_strings_extractor.process_file(_localized_strings) +    ui_strings = frozenset(strings_extractor.strings[0]) +    strings = frozenset(strings_extractor.strings[1]) +    localized_strings = frozenset(localized_strings_extractor.localized_strings) + +    new_strings = ui_strings - localized_strings +    for s in new_strings: +        _log.info("New: \"%s\"" % (s)) +    old_strings = localized_strings - ui_strings +    suspicious_strings = strings & old_strings +    for s in suspicious_strings: +        _log.info("Suspicious: \"%s\"" % (s)) +    unused_strings = old_strings - strings +    for s in unused_strings: +        _log.info("Unused: \"%s\"" % (s)) diff --git a/Tools/Scripts/do-file-rename b/Tools/Scripts/do-file-rename index 6ad9331..afb72d8 100755 --- a/Tools/Scripts/do-file-rename +++ b/Tools/Scripts/do-file-rename @@ -47,7 +47,7 @@ find(\&wanted, "Source/JavaScriptCore");  find(\&wanted, "Source/JavaScriptGlue");  find(\&wanted, "Source/WebCore");  find(\&wanted, "WebKit"); -find(\&wanted, "WebKit2"); +find(\&wanted, "Source/WebKit2");  sub wanted  { diff --git a/Tools/Scripts/do-webcore-rename b/Tools/Scripts/do-webcore-rename index 0f84ad7..aaa1eee 100755 --- a/Tools/Scripts/do-webcore-rename +++ b/Tools/Scripts/do-webcore-rename @@ -73,7 +73,7 @@ find(\&wanted, "Source/JavaScriptCore");  find(\&wanted, "Source/JavaScriptGlue");  find(\&wanted, "Source/WebCore");  find(\&wanted, "WebKit"); -find(\&wanted, "WebKit2"); +find(\&wanted, "Source/WebKit2");  find(\&wanted, "Tools/DumpRenderTree");  sub wanted diff --git a/Tools/Scripts/old-run-webkit-tests b/Tools/Scripts/old-run-webkit-tests index ab41e9b..892b5a4 100755 --- a/Tools/Scripts/old-run-webkit-tests +++ b/Tools/Scripts/old-run-webkit-tests @@ -113,7 +113,7 @@ sub splitpath($);  sub stopRunningTestsEarlyIfNeeded();  sub stripExtension($);  sub stripMetrics($$); -sub testCrashedOrTimedOut($$$$$); +sub testCrashedOrTimedOut($$$$$$);  sub toCygwinPath($);  sub toURL($);  sub toWindowsPath($); @@ -170,6 +170,7 @@ my $useValgrind = 0;  my $verbose = 0;  my $shouldWaitForHTTPD = 0;  my $useWebKitTestRunner = 0; +my $noBuildDumpTool = 0;  my @leaksFilenames; @@ -329,6 +330,7 @@ my $getOptionsResult = GetOptions(      'leaks|l' => \$shouldCheckLeaks,      'merge-leak-depth|m:5' => \$mergeDepth,      'new-test-results!' => \$generateNewResults, +    'no-build' => \$noBuildDumpTool,      'nthly=i' => \$testsPerDumpTool,      'pixel-tests|p' => \$pixelTests,      'platform=s' => \$platform, @@ -405,7 +407,7 @@ $productDir .= "/Programs" if isGtk();  chdirWebKit(); -if (!defined($root)) { +if (!defined($root) && !$noBuildDumpTool) {      # FIXME: We build both DumpRenderTree and WebKitTestRunner for      # WebKitTestRunner runs becuase DumpRenderTree still includes      # the DumpRenderTreeSupport module and the TestNetscapePlugin. @@ -446,6 +448,8 @@ my @platformTestHierarchy = buildPlatformTestHierarchy(@platformResultHierarchy)  $expectedDirectory = $ENV{"WebKitExpectedTestResultsDirectory"} if $ENV{"WebKitExpectedTestResultsDirectory"};  $testResultsDirectory = File::Spec->rel2abs($testResultsDirectory); +# $testResultsDirectory must be empty before testing. +rmtree $testResultsDirectory;  my $testResults = File::Spec->catfile($testResultsDirectory, "results.html");  if (isAppleMacWebKit()) { @@ -804,8 +808,9 @@ for my $test (@tests) {      unless ($readResults->{status} eq "success") {          my $crashed = $readResults->{status} eq "crashed"; -        testCrashedOrTimedOut($test, $base, $crashed, $actual, $error); -        countFinishedTest($test, $base, $crashed ? "crash" : "timedout", 0); +        my $webProcessCrashed = $readResults->{status} eq "webProcessCrashed"; +        testCrashedOrTimedOut($test, $base, $crashed, $webProcessCrashed, $actual, $error); +        countFinishedTest($test, $base, $webProcessCrashed ? "webProcessCrash" : $crashed ? "crash" : "timedout", 0);          last if stopRunningTestsEarlyIfNeeded();          next;      } @@ -914,7 +919,7 @@ for my $test (@tests) {      if (dumpToolDidCrash()) {          $result = "crash"; -        testCrashedOrTimedOut($test, $base, 1, $actual, $error); +        testCrashedOrTimedOut($test, $base, 1, 0, $actual, $error);      } elsif (!defined $expected) {          if ($verbose) {              print "new " . ($resetResults ? "result" : "test"); @@ -1137,6 +1142,7 @@ if ($ignoreMetrics) {  print HTML htmlForResultsSection(@{$tests{mismatch}}, "Tests where results did not match expected results", \&linksForMismatchTest);  print HTML htmlForResultsSection(@{$tests{timedout}}, "Tests that timed out", \&linksForErrorTest);  print HTML htmlForResultsSection(@{$tests{crash}}, "Tests that caused the DumpRenderTree tool to crash", \&linksForErrorTest); +print HTML htmlForResultsSection(@{$tests{webProcessCrash}}, "Tests that caused the Web process to crash", \&linksForErrorTest);  print HTML htmlForResultsSection(@{$tests{error}}, "Tests that had stderr output", \&linksForErrorTest);  print HTML htmlForResultsSection(@{$tests{new}}, "Tests that had no expected results (probably new)", \&linksForNewTest); @@ -1700,13 +1706,13 @@ sub countFinishedTest($$$$)      push @{$tests{$result}}, $test;  } -sub testCrashedOrTimedOut($$$$$) +sub testCrashedOrTimedOut($$$$$$)  { -    my ($test, $base, $didCrash, $actual, $error) = @_; +    my ($test, $base, $didCrash, $webProcessCrashed, $actual, $error) = @_; -    printFailureMessageForTest($test, $didCrash ? "crashed" : "timed out"); +    printFailureMessageForTest($test, $webProcessCrashed ? "Web process crashed" : $didCrash ? "crashed" : "timed out"); -    sampleDumpTool() unless $didCrash; +    sampleDumpTool() unless $didCrash || $webProcessCrashed;      my $dir = dirname(File::Spec->catdir($testResultsDirectory, $base));      mkpath $dir; @@ -2155,6 +2161,10 @@ sub readFromDumpToolWithTimer(**)              }          }          if (defined($lineError)) { +            if ($lineError =~ /#CRASHED - WebProcess/) { +                $status = "webProcessCrashed"; +                last; +            }              if ($lineError =~ /#CRASHED/) {                  $status = "crashed";                  last; @@ -2392,10 +2402,11 @@ sub printResults          new => "were new",          timedout => "timed out",          crash => "crashed", +        webProcessCrash => "Web process crashed",          error => "had stderr output"      ); -    for my $type ("match", "mismatch", "new", "timedout", "crash", "error") { +    for my $type ("match", "mismatch", "new", "timedout", "crash", "webProcessCrash", "error") {          my $typeCount = $counts{$type};          next unless $typeCount;          my $typeText = $text{$type}; diff --git a/Tools/Scripts/test-webkitpy b/Tools/Scripts/test-webkitpy index 7efacb0..7c11e85 100755 --- a/Tools/Scripts/test-webkitpy +++ b/Tools/Scripts/test-webkitpy @@ -248,7 +248,7 @@ def _test_import(module_path):  if __name__ == "__main__":      # FIXME: We should probably test each package separately to avoid naming conflicts.      external_package_paths = [ -        _path_from_webkit_root('WebKit2', 'Scripts', 'webkit2'), +        _path_from_webkit_root('Source', 'WebKit2', 'Scripts', 'webkit2'),          _path_from_webkit_root('Tools', 'QueueStatusServer'),      ]      init(sys.argv[1:], external_package_paths) diff --git a/Tools/Scripts/update-webkit-auxiliary-libs b/Tools/Scripts/update-webkit-auxiliary-libs index 19e4ad3..9a6b20f 100755 --- a/Tools/Scripts/update-webkit-auxiliary-libs +++ b/Tools/Scripts/update-webkit-auxiliary-libs @@ -54,11 +54,12 @@ my $file = "WebKitAuxiliaryLibrary";  my $zipFile = "$file.zip";   my $auxiliaryLibsURL = "http://developer.apple.com/opensource/internet/$zipFile";  my $webkitLibrariesDir = toUnixPath($ENV{'WEBKITLIBRARIESDIR'}) || "$sourceDir/WebKitLibraries/win"; -my $tmpDir = File::Spec->rel2abs(File::Temp::tempdir("webkitlibsXXXXXXX", TMPDIR => 1, CLEANUP => 1)); +my $tmpRelativeDir = File::Temp::tempdir("webkitlibsXXXXXXX", TMPDIR => 1, CLEANUP => 1); +my $tmpAbsDir = File::Spec->rel2abs($tmpRelativeDir);  print "Checking Last-Modified date of $zipFile...\n"; -my $result = system "curl -s -I $auxiliaryLibsURL | grep Last-Modified > \"$tmpDir/$file.headers\""; +my $result = system "curl -s -I $auxiliaryLibsURL | grep Last-Modified > \"$tmpAbsDir/$file.headers\"";  if (WEXITSTATUS($result)) {      print STDERR "Couldn't check Last-Modified date of new $zipFile.\n"; @@ -73,7 +74,7 @@ if (WEXITSTATUS($result)) {      exit 0;  } -if (open NEW, "$tmpDir/$file.headers") { +if (open NEW, "$tmpAbsDir/$file.headers") {      my $new = lastModifiedToUnixTime(<NEW>);      close NEW; @@ -88,17 +89,17 @@ if (open NEW, "$tmpDir/$file.headers") {  }  print "Downloading $zipFile...\n\n"; -$result = system "curl -o \"$tmpDir/$zipFile\" $auxiliaryLibsURL"; +$result = system "curl -o \"$tmpAbsDir/$zipFile\" $auxiliaryLibsURL";  die "Couldn't download $zipFile!" if $result; -$result = system "unzip", "-q", "-d", $tmpDir, "$tmpDir/$zipFile"; +$result = system "unzip", "-q", "-d", $tmpAbsDir, "$tmpAbsDir/$zipFile";  die "Couldn't unzip $zipFile." if $result;  print "\nInstalling $file...\n";  sub wanted  { -    my $relativeName = File::Spec->abs2rel($File::Find::name, "$tmpDir/$file/win"); +    my $relativeName = File::Spec->abs2rel($File::Find::name, "$tmpAbsDir/$file/win");      my $destination = "$webkitLibrariesDir/$relativeName";      if (-d $_) { @@ -109,9 +110,9 @@ sub wanted      system "cp", $_, $destination;  } -File::Find::find(\&wanted, "$tmpDir/$file"); +File::Find::find(\&wanted, "$tmpAbsDir/$file"); -$result = system "mv", "$tmpDir/$file.headers", $webkitLibrariesDir; +$result = system "mv", "$tmpAbsDir/$file.headers", $webkitLibrariesDir;  print STDERR "Couldn't move $file.headers to $webkitLibrariesDir" . ".\n" if $result;  print "The $file has been sucessfully installed in\n $webkitLibrariesDir\n"; diff --git a/Tools/Scripts/update-webkit-chromium b/Tools/Scripts/update-webkit-chromium index 1db1826..5610487 100755 --- a/Tools/Scripts/update-webkit-chromium +++ b/Tools/Scripts/update-webkit-chromium @@ -34,7 +34,7 @@ use Getopt::Long;  use lib $FindBin::Bin;  use webkitdirs; -chdir("WebKit/chromium") or die $!; +chdir("Source/WebKit/chromium") or die $!;  # Find gclient or install it.  my $gclientPath; diff --git a/Tools/Scripts/update-webkit-support-libs b/Tools/Scripts/update-webkit-support-libs index f0c897e..8484f84 100755 --- a/Tools/Scripts/update-webkit-support-libs +++ b/Tools/Scripts/update-webkit-support-libs @@ -49,7 +49,8 @@ my $pathToZip = File::Spec->catfile($zipDirectory, $zipFile);  my $webkitLibrariesDir = toUnixPath($ENV{'WEBKITLIBRARIESDIR'}) || "$sourceDir/WebKitLibraries/win";  my $versionFile = $file . "Version";  my $pathToVersionFile = File::Spec->catfile($webkitLibrariesDir, $versionFile); -my $tmpDir = File::Spec->rel2abs(File::Temp::tempdir("webkitlibsXXXXXXX", TMPDIR => 1, CLEANUP => 1)); +my $tmpRelativeDir = File::Temp::tempdir("webkitlibsXXXXXXX", TMPDIR => 1, CLEANUP => 1); +my $tmpAbsDir = File::Spec->rel2abs($tmpRelativeDir);  my $versionFileURL = "http://developer.apple.com/opensource/internet/$versionFile";  my $extractedVersion = extractedVersion(); @@ -70,14 +71,14 @@ if ($zipFileVersion eq $extractedVersion) {      exit;  } -my $result = system "unzip", "-q", "-d", $tmpDir, $pathToZip; +my $result = system "unzip", "-q", "-d", $tmpAbsDir, $pathToZip;  die "Couldn't unzip $zipFile." if $result;  print "\nInstalling $file...\n";  sub wanted  { -    my $relativeName = File::Spec->abs2rel($File::Find::name, "$tmpDir/$file/win"); +    my $relativeName = File::Spec->abs2rel($File::Find::name, "$tmpAbsDir/$file/win");      my $destination = "$webkitLibrariesDir/$relativeName";      if (-d $_) { @@ -88,7 +89,7 @@ sub wanted      system "cp", $_, $destination;  } -File::Find::find(\&wanted, "$tmpDir/$file"); +File::Find::find(\&wanted, "$tmpAbsDir/$file");  print "The $file has been sucessfully installed in\n $webkitLibrariesDir\n";  exit; diff --git a/Tools/Scripts/webkitdirs.pm b/Tools/Scripts/webkitdirs.pm index 2525dd7..0ead831 100644 --- a/Tools/Scripts/webkitdirs.pm +++ b/Tools/Scripts/webkitdirs.pm @@ -87,7 +87,7 @@ sub determineSourceDir      # walks up path checking each directory to see if it is the main WebKit project dir,       # defined by containing Sources, WebCore, and WebKit -    until ((-d "$sourceDir/Source" && -d "$sourceDir/Source/WebCore" && -d "$sourceDir/WebKit") || (-d "$sourceDir/Internal" && -d "$sourceDir/OpenSource")) +    until ((-d "$sourceDir/Source" && -d "$sourceDir/Source/WebCore" && -d "$sourceDir/Source/WebKit") || (-d "$sourceDir/Internal" && -d "$sourceDir/OpenSource"))      {          if ($sourceDir !~ s|/[^/]+$||) {              die "Could not find top level webkit directory above source directory using FindBin.\n"; @@ -603,10 +603,11 @@ sub builtDylibPathForName      }      if (isGtk()) {          my $libraryDir = "$configurationProductDir/.libs/"; -        if (-e $libraryDir . "libwebkitgtk-3.0.so") { -            return $libraryDir . "libwebkitgtk-3.0.so"; +        my $extension = isDarwin() ? "dylib" : "so"; +        if (-e $libraryDir . "libwebkitgtk-3.0.$extension") { +            return $libraryDir . "libwebkitgtk-3.0.$extension";          } -        return $libraryDir . "libwebkitgtk-1.0.so"; +        return $libraryDir . "libwebkitgtk-1.0.$extension";      }      if (isEfl()) {          return "$configurationProductDir/$libraryName/../.libs/libewebkit.so"; @@ -1590,9 +1591,9 @@ sub buildQMakeProject($@)      my $dsMakefile = "Makefile.DerivedSources";      # Iterate over different source directories manually to workaround a problem with qmake+extraTargets+s60 -    my @subdirs = ("Source/JavaScriptCore", "Source/WebCore", "WebKit/qt/Api"); +    my @subdirs = ("Source/JavaScriptCore", "Source/WebCore", "Source/WebKit/qt/Api");      if (grep { $_ eq "CONFIG+=webkit2"} @buildArgs) { -        push @subdirs, "WebKit2"; +        push @subdirs, "Source/WebKit2";          push @subdirs, "Tools/WebKitTestRunner";          push @subdirs, "Tools/MiniBrowser";      } @@ -1637,9 +1638,9 @@ sub buildQMakeProject($@)      }      # Manually create makefiles for the examples so we don't build by default -    my $examplesDir = $dir . "/WebKit/qt/examples"; +    my $examplesDir = $dir . "/Source/WebKit/qt/examples";      File::Path::mkpath($examplesDir); -    $buildArgs[-1] = sourceDir() . "/WebKit/qt/examples/examples.pro"; +    $buildArgs[-1] = sourceDir() . "/Source/WebKit/qt/examples/examples.pro";      chdir $examplesDir or die;      print "Calling '$qmakebin @buildArgs' in " . $examplesDir . "\n\n";      $result = system "$qmakebin @buildArgs"; @@ -1745,10 +1746,10 @@ sub buildChromium($@)      my $result = 1;      if (isDarwin()) {          # Mac build - builds the root xcode project. -        $result = buildXCodeProject("WebKit/chromium/WebKit", $clean, "-configuration", configuration(), @options); +        $result = buildXCodeProject("Source/WebKit/chromium/WebKit", $clean, "-configuration", configuration(), @options);      } elsif (isCygwin() || isWindows()) {          # Windows build - builds the root visual studio solution. -        $result = buildChromiumVisualStudioProject("WebKit/chromium/WebKit.sln", $clean); +        $result = buildChromiumVisualStudioProject("Source/WebKit/chromium/WebKit.sln", $clean);      } elsif (isLinux()) {          # Linux build - build using make.          $ result = buildChromiumMakefile("all", $clean); diff --git a/Tools/Scripts/webkitpy/common/checkout/api.py b/Tools/Scripts/webkitpy/common/checkout/api.py index 29e43d3..a87bb5a 100644 --- a/Tools/Scripts/webkitpy/common/checkout/api.py +++ b/Tools/Scripts/webkitpy/common/checkout/api.py @@ -54,12 +54,25 @@ class Checkout(object):          # contents_at_revision returns a byte array (str()), but we know          # that ChangeLog files are utf-8.  parse_latest_entry_from_file          # expects a file-like object which vends unicode(), so we decode here. -        changelog_file = StringIO.StringIO(changelog_contents.decode("utf-8")) +        # Old revisions of Sources/WebKit/wx/ChangeLog have some invalid utf8 characters. +        changelog_file = StringIO.StringIO(changelog_contents.decode("utf-8", "ignore"))          return ChangeLog.parse_latest_entry_from_file(changelog_file)      def changelog_entries_for_revision(self, revision):          changed_files = self._scm.changed_files_for_revision(revision) -        return [self._latest_entry_for_changelog_at_revision(path, revision) for path in changed_files if self.is_path_to_changelog(path)] +        # FIXME: This gets confused if ChangeLog files are moved, as +        # deletes are still "changed files" per changed_files_for_revision. +        # FIXME: For now we hack around this by caching any exceptions +        # which result from having deleted files included the changed_files list. +        changelog_entries = [] +        for path in changed_files: +            if not self.is_path_to_changelog(path): +                continue +            try: +                changelog_entries.append(self._latest_entry_for_changelog_at_revision(path, revision)) +            except ScriptError: +                pass +        return changelog_entries      @memoized      def commit_info_for_revision(self, revision): diff --git a/Tools/Scripts/webkitpy/common/checkout/api_unittest.py b/Tools/Scripts/webkitpy/common/checkout/api_unittest.py index 1f97abd..fdf3fba 100644 --- a/Tools/Scripts/webkitpy/common/checkout/api_unittest.py +++ b/Tools/Scripts/webkitpy/common/checkout/api_unittest.py @@ -38,6 +38,7 @@ from webkitpy.common.checkout.api import Checkout  from webkitpy.common.checkout.changelog import ChangeLogEntry  from webkitpy.common.checkout.scm import detect_scm_system, CommitMessage  from webkitpy.common.system.outputcapture import OutputCapture +from webkitpy.common.system.executive import ScriptError  from webkitpy.thirdparty.mock import Mock @@ -130,12 +131,35 @@ class CheckoutTest(unittest.TestCase):              self.assertEqual(revision, "bar")              # contents_at_revision is expected to return a byte array (str)              # so we encode our unicode ChangeLog down to a utf-8 stream. -            return _changelog1.encode("utf-8") +            # The ChangeLog utf-8 decoding should ignore invalid codepoints. +            invalid_utf8 = "\255" +            return _changelog1.encode("utf-8") + invalid_utf8          scm.contents_at_revision = mock_contents_at_revision          checkout = Checkout(scm)          entry = checkout._latest_entry_for_changelog_at_revision("foo", "bar")          self.assertEqual(entry.contents(), _changelog1entry1) +    # FIXME: This tests a hack around our current changed_files handling. +    # Right now changelog_entries_for_revision tries to fetch deleted files +    # from revisions, resulting in a ScriptError exception.  Test that we +    # recover from those and still return the other ChangeLog entries. +    def test_changelog_entries_for_revision(self): +        scm = Mock() +        scm.changed_files_for_revision = lambda revision: ['foo/ChangeLog', 'bar/ChangeLog'] +        checkout = Checkout(scm) + +        def mock_latest_entry_for_changelog_at_revision(path, revision): +            if path == "foo/ChangeLog": +                return 'foo' +            raise ScriptError() + +        checkout._latest_entry_for_changelog_at_revision = mock_latest_entry_for_changelog_at_revision + +        # Even though fetching one of the entries failed, the other should succeed. +        entries = checkout.changelog_entries_for_revision(1) +        self.assertEqual(len(entries), 1) +        self.assertEqual(entries[0], 'foo') +      def test_commit_info_for_revision(self):          scm = Mock()          scm.committer_email_for_revision = lambda revision: "committer@example.com" diff --git a/Tools/Scripts/webkitpy/common/checkout/scm.py b/Tools/Scripts/webkitpy/common/checkout/scm.py index 3f77043..421c0dc 100644 --- a/Tools/Scripts/webkitpy/common/checkout/scm.py +++ b/Tools/Scripts/webkitpy/common/checkout/scm.py @@ -36,6 +36,7 @@ import shutil  from webkitpy.common.system.executive import Executive, run_command, ScriptError  from webkitpy.common.system.deprecated_logging import error, log +import webkitpy.common.system.ospath as ospath  from webkitpy.common.memoized import memoized @@ -52,7 +53,7 @@ def find_checkout_root():      return None -def default_scm(): +def default_scm(patch_directories=None):      """Return the default SCM object as determined by the CWD and running code.      Returns the default SCM object for the current working directory; if the @@ -62,10 +63,10 @@ def default_scm():      """      cwd = os.getcwd() -    scm_system = detect_scm_system(cwd) +    scm_system = detect_scm_system(cwd, patch_directories)      if not scm_system:          script_directory = os.path.dirname(os.path.abspath(__file__)) -        scm_system = detect_scm_system(script_directory) +        scm_system = detect_scm_system(script_directory, patch_directories)          if scm_system:              log("The current directory (%s) is not a WebKit checkout, using %s" % (cwd, scm_system.checkout_root))          else: @@ -73,11 +74,14 @@ def default_scm():      return scm_system -def detect_scm_system(path): +def detect_scm_system(path, patch_directories=None):      absolute_path = os.path.abspath(path) +    if patch_directories == []: +        patch_directories = None +      if SVN.in_working_directory(absolute_path): -        return SVN(cwd=absolute_path) +        return SVN(cwd=absolute_path, patch_directories=patch_directories)      if Git.in_working_directory(absolute_path):          return Git(cwd=absolute_path) @@ -141,16 +145,16 @@ class AmbiguousCommitError(Exception):  # SCM methods are expected to return paths relative to self.checkout_root.  class SCM: -    def __init__(self, cwd): +    def __init__(self, cwd, executive=None):          self.cwd = cwd          self.checkout_root = self.find_checkout_root(self.cwd)          self.dryrun = False +        self._executive = executive or Executive()      # A wrapper used by subclasses to create processes.      def run(self, args, cwd=None, input=None, error_handler=None, return_exit_code=False, return_stderr=True, decode_output=True):          # FIXME: We should set cwd appropriately. -        # FIXME: We should use Executive. -        return run_command(args, +        return self._executive.run_command(args,                             cwd=cwd,                             input=input,                             error_handler=error_handler, @@ -262,7 +266,7 @@ class SCM:      def display_name(self):          self._subclass_must_implement() -    def create_patch(self, git_commit=None, changed_files=[]): +    def create_patch(self, git_commit=None, changed_files=None):          self._subclass_must_implement()      def committer_email_for_revision(self, revision): @@ -315,13 +319,20 @@ class SCM:  class SVN(SCM): -    # FIXME: We should move these values to a WebKit-specific config. file. +    # FIXME: We should move these values to a WebKit-specific config file.      svn_server_host = "svn.webkit.org"      svn_server_realm = "<http://svn.webkit.org:80> Mac OS Forge" -    def __init__(self, cwd): -        SCM.__init__(self, cwd) +    def __init__(self, cwd, patch_directories, executive=None): +        SCM.__init__(self, cwd, executive)          self._bogus_dir = None +        if patch_directories == []: +            # FIXME: ScriptError is for Executive, this should probably be a normal Exception. +            raise ScriptError(script_args=svn_info_args, message='Empty list of patch directories passed to SCM.__init__') +        elif patch_directories == None: +            self._patch_directories = [ospath.relpath(cwd, self.checkout_root)] +        else: +            self._patch_directories = patch_directories      @staticmethod      def in_working_directory(path): @@ -427,7 +438,10 @@ class SVN(SCM):          return self.run(["svn", "delete", "--force", base], cwd=parent)      def changed_files(self, git_commit=None): -        return self.run_status_and_extract_filenames(self.status_command(), self._status_regexp("ACDMR")) +        status_command = ["svn", "status"] +        status_command.extend(self._patch_directories) +        # ACDMR: Addded, Conflicted, Deleted, Modified or Replaced +        return self.run_status_and_extract_filenames(status_command, self._status_regexp("ACDMR"))      def changed_files_for_revision(self, revision):          # As far as I can tell svn diff --summarize output looks just like svn status output. @@ -463,10 +477,14 @@ class SVN(SCM):          return "svn"      # FIXME: This method should be on Checkout. -    def create_patch(self, git_commit=None, changed_files=[]): +    def create_patch(self, git_commit=None, changed_files=None):          """Returns a byte array (str()) representing the patch file.          Patch files are effectively binary since they may contain          files of multiple different encodings.""" +        if changed_files == []: +            return "" +        elif changed_files == None: +            changed_files = []          return self.run([self.script_path("svn-create-patch")] + changed_files,              cwd=self.checkout_root, return_stderr=False,              decode_output=False) @@ -574,8 +592,8 @@ class SVN(SCM):  # All git-specific logic should go here.  class Git(SCM): -    def __init__(self, cwd): -        SCM.__init__(self, cwd) +    def __init__(self, cwd, executive=None): +        SCM.__init__(self, cwd, executive)          self._check_git_architecture()      def _machine_is_64bit(self): @@ -688,7 +706,10 @@ class Git(SCM):          return self.remote_merge_base()      def changed_files(self, git_commit=None): +        # FIXME: --diff-filter could be used to avoid the "extract_filenames" step.          status_command = ['git', 'diff', '-r', '--name-status', '-C', '-M', "--no-ext-diff", "--full-index", self.merge_base(git_commit)] +        # FIXME: I'm not sure we're returning the same set of files that SVN.changed_files is. +        # Added (A), Copied (C), Deleted (D), Modified (M), Renamed (R)          return self.run_status_and_extract_filenames(status_command, self._status_regexp("ADM"))      def _changes_files_for_commit(self, git_commit): @@ -725,11 +746,14 @@ class Git(SCM):      def display_name(self):          return "git" -    def create_patch(self, git_commit=None, changed_files=[]): +    def create_patch(self, git_commit=None, changed_files=None):          """Returns a byte array (str()) representing the patch file.          Patch files are effectively binary since they may contain          files of multiple different encodings.""" -        return self.run(['git', 'diff', '--binary', "--no-ext-diff", "--full-index", "-M", self.merge_base(git_commit), "--"] + changed_files, decode_output=False, cwd=self.checkout_root) +        command = ['git', 'diff', '--binary', "--no-ext-diff", "--full-index", "-M", self.merge_base(git_commit), "--"] +        if changed_files: +            command += changed_files +        return self.run(command, decode_output=False, cwd=self.checkout_root)      def _run_git_svn_find_rev(self, arg):          # git svn find-rev always exits 0, even when the revision or commit is not found. diff --git a/Tools/Scripts/webkitpy/common/checkout/scm_unittest.py b/Tools/Scripts/webkitpy/common/checkout/scm_unittest.py index 8f24beb..64122b4 100644 --- a/Tools/Scripts/webkitpy/common/checkout/scm_unittest.py +++ b/Tools/Scripts/webkitpy/common/checkout/scm_unittest.py @@ -45,11 +45,12 @@ 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, AuthenticationError, AmbiguousCommitError, find_checkout_root, default_scm +from webkitpy.common.checkout.scm import detect_scm_system, SCM, SVN, Git, CheckoutNeedsUpdate, commit_error_handler, AuthenticationError, AmbiguousCommitError, find_checkout_root, default_scm  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  from webkitpy.common.system.outputcapture import OutputCapture +from webkitpy.tool.mocktool import MockExecutive  # Eventually we will want to write tests which work for both scms. (like update_webkit, changed_files, etc.)  # Perhaps through some SCMTest base-class which both SVNTest and GitTest inherit from. @@ -456,6 +457,7 @@ OcmYex&reD$;sO8*F9L)B          self.scm.add("added_dir/added_file")          self.assertTrue("added_dir/added_file" in self.scm.added_files()) +  class SVNTest(SCMTest):      @staticmethod @@ -1316,5 +1318,20 @@ class GitSVNTest(SCMTest):          self.assertTrue("+Updated" in cached_diff)          self.assertTrue("-more test content" in cached_diff) + +# We need to split off more of these SCM tests to use mocks instead of the filesystem. +# This class is the first part of that. +class GitTestWithMock(unittest.TestCase): +    def setUp(self): +        executive = MockExecutive(should_log=False) +        # We do this should_log dance to avoid logging when Git.__init__ runs sysctl on mac to check for 64-bit support. +        self.scm = Git(None, executive=executive) +        executive.should_log = True + +    def test_create_patch(self): +        expected_stderr = "MOCK run_command: ['git', 'merge-base', u'refs/remotes/origin/master', 'HEAD']\nMOCK run_command: ['git', 'diff', '--binary', '--no-ext-diff', '--full-index', '-M', 'MOCK output of child process', '--']\n" +        OutputCapture().assert_outputs(self, self.scm.create_patch, kwargs={'changed_files': None}, expected_stderr=expected_stderr) + +  if __name__ == '__main__':      unittest.main() diff --git a/Tools/Scripts/webkitpy/common/config/committers.py b/Tools/Scripts/webkitpy/common/config/committers.py index c7c741b..6a235f5 100644 --- a/Tools/Scripts/webkitpy/common/config/committers.py +++ b/Tools/Scripts/webkitpy/common/config/committers.py @@ -106,6 +106,7 @@ committers_unable_to_review = [      Committer("Enrica Casucci", "enrica@apple.com"),      Committer("Erik Arvidsson", "arv@chromium.org", "arv"),      Committer("Eric Roman", "eroman@chromium.org", "eroman"), +    Committer("Eric Uhrhane", "ericu@chromium.org", "ericu"),      Committer("Evan Martin", "evan@chromium.org", "evmar"),      Committer("Evan Stade", "estade@chromium.org", "estade"),      Committer("Fady Samuel", "fsamuel@chromium.org", "fsamuel"), @@ -134,6 +135,7 @@ committers_unable_to_review = [      Committer("Jochen Eisinger", "jochen@chromium.org", "jochen__"),      Committer("John Abd-El-Malek", "jam@chromium.org", "jam"),      Committer("John Gregg", ["johnnyg@google.com", "johnnyg@chromium.org"], "johnnyg"), +    Committer("John Knottenbelt", ["jknotten@chromium.org"], "jknotten"),      Committer("Johnny Ding", ["jnd@chromium.org", "johnnyding.webkit@gmail.com"], "johnnyding"),      Committer("Joost de Valk", ["joost@webkit.org", "webkit-dev@joostdevalk.nl"], "Altha"),      Committer("Julie Parent", ["jparent@google.com", "jparent@chromium.org"], "jparent"), @@ -173,6 +175,7 @@ committers_unable_to_review = [      Committer("Patrick Gansterer", ["paroga@paroga.com", "paroga@webkit.org"], "paroga"),      Committer("Pavel Podivilov", "podivilov@chromium.org", "podivilov"),      Committer("Peter Kasting", ["pkasting@google.com", "pkasting@chromium.org"], "pkasting"), +    Committer("Peter Varga", ["pvarga@webkit.org", "pvarga@inf.u-szeged.hu"], "stampho"),      Committer("Philippe Normand", ["pnormand@igalia.com", "philn@webkit.org"], "philn-tp"),      Committer("Pierre d'Herbemont", ["pdherbemont@free.fr", "pdherbemont@apple.com"], "pdherbemont"),      Committer("Pierre-Olivier Latour", "pol@apple.com", "pol"), @@ -184,7 +187,6 @@ committers_unable_to_review = [      Committer("Scott Violet", "sky@chromium.org", "sky"),      Committer("Sergio Villar Senin", ["svillar@igalia.com", "sergio@webkit.org"], "svillar"),      Committer("Stephen White", "senorblanco@chromium.org", "senorblanco"), -    Committer("Tony Gentilcore", "tonyg@chromium.org", "tonyg-cr"),      Committer("Trey Matteson", "trey@usa.net", "trey"),      Committer("Tristan O'Tierney", ["tristan@otierney.net", "tristan@apple.com"]),      Committer("Vangelis Kokkevis", "vangelis@chromium.org", "vangelis"), @@ -290,6 +292,7 @@ reviewers_list = [      Reviewer("Tim Omernick", "timo@apple.com"),      Reviewer("Timothy Hatcher", ["timothy@apple.com", "timothy@hatcher.name"], "xenon"),      Reviewer("Tony Chang", "tony@chromium.org", "tony^work"), +    Reviewer("Tony Gentilcore", "tonyg@chromium.org", "tonyg-cr"),      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"), diff --git a/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py b/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py index d6210d5..17a8515 100644 --- a/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py +++ b/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py @@ -30,6 +30,7 @@  #  # WebKit's Python module for interacting with Bugzilla +import mimetypes  import os.path  import re  import StringIO @@ -441,7 +442,11 @@ class Bugzilla(object):          self.browser['flag_type-3'] = (self._commit_queue_flag(mark_for_landing, mark_for_commit_queue),)          filename = filename or "%s.patch" % timestamp() -        mimetype = mimetype or "text/plain" +        if not mimetype: +            mimetypes.add_type('text/plain', '.patch')  # Make sure mimetypes knows about .patch +            mimetype, _ = mimetypes.guess_type(filename) +        if not mimetype: +            mimetype = "text/plain"  # Bugzilla might auto-guess for us and we might not need this?          self.browser.add_file(file_object, mimetype, filename, 'data')      def _file_object_for_upload(self, file_or_string): diff --git a/Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py b/Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py index 98e2fae..3cb6da5 100644 --- a/Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py +++ b/Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py @@ -270,7 +270,6 @@ class BuildBot(object):              "Leopard",              "Tiger",              "Windows.*Build", -            "EFL",              "GTK.*32",              "GTK.*64.*Debug",  # Disallow the 64-bit Release bot which is broken.              "Qt", diff --git a/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py b/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py index ba898ec..57290d1 100644 --- a/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py +++ b/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py @@ -259,7 +259,6 @@ class BuildBotTest(unittest.TestCase):              "Leopard",              "Tiger",              "Windows.*Build", -            "EFL",              "GTK.*32",              "GTK.*64.*Debug",  # Disallow the 64-bit Release bot which is broken.              "Qt", diff --git a/Tools/Scripts/webkitpy/common/system/executive.py b/Tools/Scripts/webkitpy/common/system/executive.py index 85a683a..02619db 100644 --- a/Tools/Scripts/webkitpy/common/system/executive.py +++ b/Tools/Scripts/webkitpy/common/system/executive.py @@ -53,6 +53,14 @@ _log = logging.getLogger("webkitpy.common.system")  class ScriptError(Exception): +    # This is a custom List.__str__ implementation to allow size limiting. +    def _string_from_args(self, args, limit=100): +        args_string = unicode(args) +        # We could make this much fancier, but for now this is OK. +        if len(args_string) > limit: +            return args_string[:limit - 3] + "..." +        return args_string +      def __init__(self,                   message=None,                   script_args=None, @@ -60,7 +68,7 @@ class ScriptError(Exception):                   output=None,                   cwd=None):          if not message: -            message = 'Failed to run "%s"' % script_args +            message = 'Failed to run "%s"' % self._string_from_args(script_args)              if exit_code:                  message += " exit_code: %d" % exit_code              if cwd: @@ -75,9 +83,9 @@ class ScriptError(Exception):      def message_with_output(self, output_limit=500):          if self.output:              if output_limit and len(self.output) > output_limit: -                return u"%s\nLast %s characters of output:\n%s" % \ +                return u"%s\n\nLast %s characters of output:\n%s" % \                      (self, output_limit, self.output[-output_limit:]) -            return u"%s\n%s" % (self, self.output) +            return u"%s\n\n%s" % (self, self.output)          return unicode(self)      def command_name(self): diff --git a/Tools/Scripts/webkitpy/common/system/executive_unittest.py b/Tools/Scripts/webkitpy/common/system/executive_unittest.py index b8fd82e..a43b4dc 100644 --- a/Tools/Scripts/webkitpy/common/system/executive_unittest.py +++ b/Tools/Scripts/webkitpy/common/system/executive_unittest.py @@ -37,6 +37,23 @@ from webkitpy.common.system.executive import Executive, run_command, ScriptError  from webkitpy.test import cat, echo +class ScriptErrorTest(unittest.TestCase): +    def test_string_from_args(self): +        error = ScriptError() +        self.assertEquals(error._string_from_args(None), 'None') +        self.assertEquals(error._string_from_args([]), '[]') +        self.assertEquals(error._string_from_args(map(str, range(30))), "['0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11', '12', '13', '14', '15', '16', '17'...") + +    def test_message_with_output(self): +        error = ScriptError('My custom message!', '', -1) +        self.assertEquals(error.message_with_output(), 'My custom message!') +        error = ScriptError('My custom message!', '', -1, 'My output.') +        self.assertEquals(error.message_with_output(), 'My custom message!\n\nMy output.') +        error = ScriptError('', 'my_command!', -1, 'My output.', '/Users/username/blah') +        self.assertEquals(error.message_with_output(), 'Failed to run "my_command!" exit_code: -1 cwd: /Users/username/blah\n\nMy output.') +        error = ScriptError('', 'my_command!', -1, 'ab' + '1' * 499) +        self.assertEquals(error.message_with_output(), 'Failed to run "my_command!" exit_code: -1\n\nLast 500 characters of output:\nb' + '1' * 499) +  def never_ending_command():      """Arguments for a command that will never end (useful for testing process      killing). It should be a process that is unlikely to already be running diff --git a/Tools/Scripts/webkitpy/common/system/filesystem.py b/Tools/Scripts/webkitpy/common/system/filesystem.py index 8830ea8..527b6bd 100644 --- a/Tools/Scripts/webkitpy/common/system/filesystem.py +++ b/Tools/Scripts/webkitpy/common/system/filesystem.py @@ -124,6 +124,9 @@ class FileSystem(object):                  if retry_timeout_sec < 0:                      raise e +    def remove_tree(self, path, ignore_errors=False): +        shutil.rmtree(path, ignore_errors) +      def read_binary_file(self, path):          """Return the contents of the file at the given path as a byte string."""          with file(path, 'rb') as f: diff --git a/Tools/Scripts/webkitpy/common/system/filesystem_mock.py b/Tools/Scripts/webkitpy/common/system/filesystem_mock.py index c605cb2..809c4c6 100644 --- a/Tools/Scripts/webkitpy/common/system/filesystem_mock.py +++ b/Tools/Scripts/webkitpy/common/system/filesystem_mock.py @@ -116,3 +116,6 @@ class MockFileSystem(object):      def remove(self, path):          del self.files[path] + +    def remove_tree(self, path, ignore_errors=False): +        self.files = [file for file in self.files if not file.startswith(path)] diff --git a/Tools/Scripts/webkitpy/common/system/workspace.py b/Tools/Scripts/webkitpy/common/system/workspace.py new file mode 100644 index 0000000..3b755ad --- /dev/null +++ b/Tools/Scripts/webkitpy/common/system/workspace.py @@ -0,0 +1,61 @@ +# 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. + +# A home for file logic which should sit above FileSystem, but +# below more complicated objects. + +import zipfile + +class Workspace(object): +    def __init__(self, filesystem, executive): +        self._filesystem = filesystem +        self._executive = executive  # FIXME: Remove if create_zip is moved to python. + +    def find_unused_filename(self, directory, name, extension, search_limit=10): +        for count in range(search_limit): +            if count: +                target_name = "%s-%s.%s" % (name, count, extension) +            else: +                target_name = "%s.%s" % (name, extension) +            target_path = self._filesystem.join(directory, target_name) +            if not self._filesystem.exists(target_path): +                return target_path +        # If we can't find an unused name in search_limit tries, just give up. +        return None + +    def create_zip(self, zip_path, source_path, zip_class=zipfile.ZipFile): +        # It's possible to create zips with Python: +        # zip_file = ZipFile(zip_path, 'w') +        # for root, dirs, files in os.walk(source_path): +        #     for path in files: +        #         absolute_path = os.path.join(root, path) +        #         zip_file.write(os.path.relpath(path, source_path)) +        # However, getting the paths, encoding and compression correct could be non-trivial. +        # So, for now we depend on the environment having "zip" installed (likely fails on Win32) +        self._executive.run_command(['zip', '-r', zip_path, source_path]) +        return zip_class(zip_path) diff --git a/Tools/Scripts/webkitpy/common/system/workspace_unittest.py b/Tools/Scripts/webkitpy/common/system/workspace_unittest.py new file mode 100644 index 0000000..e5fbb26 --- /dev/null +++ b/Tools/Scripts/webkitpy/common/system/workspace_unittest.py @@ -0,0 +1,55 @@ +# Copyright (C) 2010 Google Inc. All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +#    * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +#    * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +#    * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +import unittest + +from webkitpy.common.system.filesystem_mock import MockFileSystem +from webkitpy.common.system.outputcapture import OutputCapture +from webkitpy.common.system.workspace import Workspace +from webkitpy.tool.mocktool import MockExecutive + + +class WorkspaceTest(unittest.TestCase): + +    def test_find_unused_filename(self): +        filesystem = MockFileSystem({ +            "dir/foo.jpg": "", +            "dir/foo-1.jpg": "", +        }) +        workspace = Workspace(filesystem, None) +        self.assertEqual(workspace.find_unused_filename("bar", "bar", "bar"), "bar/bar.bar") +        self.assertEqual(workspace.find_unused_filename("dir", "foo", "jpg"), "dir/foo-2.jpg") + +    def test_create_zip(self): +        workspace = Workspace(None, MockExecutive(should_log=True)) +        expected_stderr = "MOCK run_command: ['zip', '-r', '/zip/path', '/source/path']\n" +        class MockZipFile(object): +            def __init__(self, path): +                self.filename = path +        archive = OutputCapture().assert_outputs(self, workspace.create_zip, ["/zip/path", "/source/path", MockZipFile], expected_stderr=expected_stderr) +        self.assertEqual(archive.filename, "/zip/path") diff --git a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py index 5dd0114..2b8190b 100644 --- a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py +++ b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py @@ -187,11 +187,11 @@ class FailureTimeout(TestFailure):  class FailureCrash(TestFailure): -    """Test shell crashed.""" +    """DumpRenderTree crashed."""      @staticmethod      def message(): -        return "Test shell crashed" +        return "DumpRenderTree crashed"      def result_html_output(self, filename):          # FIXME: create a link to the minidump file diff --git a/Tools/Scripts/webkitpy/layout_tests/port/chromium.py b/Tools/Scripts/webkitpy/layout_tests/port/chromium.py index b90421a..7e934a8 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/chromium.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/chromium.py @@ -392,6 +392,8 @@ class ChromiumDriver(base.Driver):                  cmd.append('--enable-accelerated-compositing')              if self._port.get_option('accelerated_2d_canvas'):                  cmd.append('--enable-accelerated-2d-canvas') +            if self._port.get_option('enable_hardware_gpu'): +                cmd.append('--enable-hardware-gpu')          return cmd      def start(self): diff --git a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py index c431765..a141661 100755 --- a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py +++ b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py @@ -243,6 +243,10 @@ def parse_args(args=None):              action="store_false",              dest="accelerated_2d_canvas",              help="Don't use hardware-accelerated 2D Canvas calls"), +        optparse.make_option("--enable-hardware-gpu", +            action="store_true", +            default=False, +            help="Run graphics tests on real GPU hardware vs software"),      ]      # Missing Mac-specific old-run-webkit-tests options: diff --git a/Tools/Scripts/webkitpy/style/checker.py b/Tools/Scripts/webkitpy/style/checker.py index 3cfa1c9..ebcf326 100644 --- a/Tools/Scripts/webkitpy/style/checker.py +++ b/Tools/Scripts/webkitpy/style/checker.py @@ -165,20 +165,20 @@ _PATH_RULES_SPECIFIER = [      # WebKit2 doesn't use config.h, and certain directories have other      # idiosyncracies.      ([# NPAPI has function names with underscores. -      "WebKit2/WebProcess/Plugins/Netscape"], +      "Source/WebKit2/WebProcess/Plugins/Netscape"],       ["-build/include_order",        "-readability/naming"]),      ([# The WebKit2 C API has names with underscores and whitespace-aligned        # struct members. Also, we allow unnecessary parameter names in        # WebKit2 APIs because we're matching CF's header style. -      "WebKit2/UIProcess/API/C/", -      "WebKit2/WebProcess/InjectedBundle/API/c/"], +      "Source/WebKit2/UIProcess/API/C/", +      "Source/WebKit2/WebProcess/InjectedBundle/API/c/"],       ["-build/include_order",        "-readability/naming",        "-readability/parameter_name",        "-whitespace/declaration"]),      ([# Nothing in WebKit2 uses config.h. -      "WebKit2/"], +      "Source/WebKit2/"],       ["-build/include_order"]),      # For third-party Python code, keep only the following checks-- diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp.py b/Tools/Scripts/webkitpy/style/checkers/cpp.py index 4ea7d69..250b9ee 100644 --- a/Tools/Scripts/webkitpy/style/checkers/cpp.py +++ b/Tools/Scripts/webkitpy/style/checkers/cpp.py @@ -344,6 +344,12 @@ class Position(object):          self.row = row          self.column = column +    def __str__(self): +        return '(%s, %s)' % (self.row, self.column) + +    def __cmp__(self, other): +        return self.row.__cmp__(other.row) or self.column.__cmp__(other.column) +  class Parameter(object):      """Information about one function parameter.""" @@ -486,32 +492,37 @@ class _FunctionState(object):          self.current_function = ''          self.in_a_function = False          self.lines_in_function = 0 -        # Make sure these will not be mistaken for real lines (even when a +        # Make sure these will not be mistaken for real positions (even when a          # small amount is added to them). -        self.body_start_line_number = -1000 -        self.ending_line_number = -1000 +        self.body_start_position = Position(-1000, 0) +        self.end_position = Position(-1000, 0) -    def begin(self, function_name, body_start_line_number, ending_line_number, is_declaration, +    def begin(self, function_name, function_name_start_position, body_start_position, end_position,                parameter_start_position, parameter_end_position, clean_lines):          """Start analyzing function body.          Args:              function_name: The name of the function being tracked. -            body_start_line_number: The line number of the { or the ; for a protoype. -            ending_line_number: The line number where the function ends. -            is_declaration: True if this is a prototype. -            parameter_start_position: position in elided of the '(' for the parameters. -            parameter_end_position: position in elided of the ')' for the parameters. +            function_name_start_position: Position in elided where the function name starts. +            body_start_position: Position in elided of the { or the ; for a prototype. +            end_position: Position in elided just after the final } (or ; is. +            parameter_start_position: Position in elided of the '(' for the parameters. +            parameter_end_position: Position in elided just after the ')' for the parameters.              clean_lines: A CleansedLines instance containing the file.          """          self.in_a_function = True          self.lines_in_function = -1  # Don't count the open brace line.          self.current_function = function_name -        self.body_start_line_number = body_start_line_number -        self.ending_line_number = ending_line_number -        self.is_declaration = is_declaration +        self.function_name_start_position = function_name_start_position +        self.body_start_position = body_start_position +        self.end_position = end_position +        self.is_declaration = clean_lines.elided[body_start_position.row][body_start_position.column] == ';'          self.parameter_start_position = parameter_start_position          self.parameter_end_position = parameter_end_position +        self.is_pure = False +        if self.is_declaration: +            characters_after_parameters = SingleLineView(clean_lines.elided, parameter_end_position, body_start_position).single_line +            self.is_pure = bool(match(r'\s*=\s*0\s*', characters_after_parameters))          self._clean_lines = clean_lines          self._parameter_list = None @@ -524,7 +535,7 @@ class _FunctionState(object):      def count(self, line_number):          """Count line in current function body.""" -        if self.in_a_function and line_number >= self.body_start_line_number: +        if self.in_a_function and line_number >= self.body_start_position.row:              self.lines_in_function += 1      def check(self, error, line_number): @@ -791,49 +802,58 @@ class CleansedLines(object):          return elided -def close_expression(clean_lines, line_number, pos): +def close_expression(elided, position):      """If input points to ( or { or [, finds the position that closes it. -    If clean_lines.elided[line_number][pos] points to a '(' or '{' or '[', finds -    the line_number/pos that correspond to the closing of the expression. +    If elided[position.row][position.column] points to a '(' or '{' or '[', +    finds the line_number/pos that correspond to the closing of the expression. -    Args: -      clean_lines: A CleansedLines instance containing the file. -      line_number: The number of the line to check. -      pos: A position on the line. +     Args: +       elided: A CleansedLines.elided instance containing the file. +       position: The position of the opening item. -    Returns: -      A tuple (line, line_number, pos) pointer *past* the closing brace, or -      ('', len(clean_lines.elided), -1) if we never find a close.  Note we -      ignore strings and comments when matching; and the line we return is the -      'cleansed' line at line_number. +     Returns: +      The Position *past* the closing brace, or Position(len(elided), -1) +      if we never find a close. Note we ignore strings and comments when matching.      """ - -    line = clean_lines.elided[line_number] -    start_character = line[pos] -    if start_character not in '({[': -        return (line, clean_lines.num_lines(), -1) +    line = elided[position.row] +    start_character = line[position.column]      if start_character == '(': -        end_character = ')' -    if start_character == '[': -        end_character = ']' -    if start_character == '{': -        end_character = '}' - -    num_open = line.count(start_character) - line.count(end_character) -    while num_open > 0: +        enclosing_character_regex = r'[\(\)]' +    elif start_character == '[': +        enclosing_character_regex = r'[\[\]]' +    elif start_character == '{': +        enclosing_character_regex = r'[\{\}]' +    else: +        return Position(len(elided), -1) + +    current_column = position.column + 1 +    line_number = position.row +    net_open = 1 +    for line in elided[position.row:]: +        line = line[current_column:] + +        # Search the current line for opening and closing characters. +        while True: +            next_enclosing_character = search(enclosing_character_regex, line) +            # No more on this line. +            if not next_enclosing_character: +                break +            current_column += next_enclosing_character.end(0) +            line = line[next_enclosing_character.end(0):] +            if next_enclosing_character.group(0) == start_character: +                net_open += 1 +            else: +                net_open -= 1 +                if not net_open: +                    return Position(line_number, current_column) + +        # Proceed to the next line.          line_number += 1 -        if line_number >= clean_lines.num_lines(): -            return ('', len(clean_lines.elided), -1) -        line = clean_lines.elided[line_number] -        num_open += line.count(start_character) - line.count(end_character) -    # OK, now find the end_character that actually got us back to even -    endpos = len(line) -    while num_open >= 0: -        endpos = line.rfind(')', 0, endpos) -        num_open -= 1                 # chopped off another ) -    return (line, line_number, endpos + 1) +        current_column = 0 +    # The given item was not closed. +    return Position(len(elided), -1)  def check_for_copyright(lines, error):      """Logs an error if no Copyright message appears at the top of the file.""" @@ -1378,7 +1398,7 @@ def detect_functions(clean_lines, line_number, function_state, error):        error: The function to call with any errors found.      """      # Are we now past the end of a function? -    if function_state.ending_line_number + 1 == line_number: +    if function_state.end_position.row + 1 == line_number:          function_state.end()      # If we're in a function, don't try to detect a new one. @@ -1409,7 +1429,10 @@ def detect_functions(clean_lines, line_number, function_state, error):      for start_line_number in xrange(line_number, clean_lines.num_lines()):          start_line = clean_lines.elided[start_line_number]          joined_line += ' ' + start_line.lstrip() -        if search(r'{|;', start_line): +        body_match = search(r'{|;', start_line) +        if body_match: +            body_start_position = Position(start_line_number, body_match.start(0)) +              # Replace template constructs with _ so that no spaces remain in the function name,              # while keeping the column numbers of other characters the same as "line".              line_with_no_templates = iteratively_replace_matches_with_char(r'<[^<>]*>', '_', line) @@ -1420,6 +1443,7 @@ def detect_functions(clean_lines, line_number, function_state, error):              # Use the column numbers from the modified line to find the              # function name in the original line.              function = line[match_function.start(1):match_function.end(1)] +            function_name_start_position = Position(line_number, match_function.start(1))              if match(r'TEST', function):    # Handle TEST... macros                  parameter_regexp = search(r'(\(.*\))', joined_line) @@ -1429,19 +1453,21 @@ def detect_functions(clean_lines, line_number, function_state, error):                  function += '()'              parameter_start_position = Position(line_number, match_function.end(1)) -            close_result = close_expression(clean_lines, line_number, parameter_start_position.column) -            if close_result[1] == len(clean_lines.elided): +            parameter_end_position = close_expression(clean_lines.elided, parameter_start_position) +            if parameter_end_position.row == len(clean_lines.elided):                  # No end was found.                  return -            parameter_end_position = Position(close_result[1], close_result[2]) -            is_declaration = bool(search(r'^[^{]*;', start_line)) -            if is_declaration: -                ending_line_number = start_line_number +            if start_line[body_start_position.column] == ';': +                end_position = Position(body_start_position.row, body_start_position.column + 1)              else: -                open_brace_index = start_line.find('{') -                ending_line_number = close_expression(clean_lines, start_line_number, open_brace_index)[1] -            function_state.begin(function, start_line_number, ending_line_number, is_declaration, +                end_position = close_expression(clean_lines.elided, body_start_position) + +            # Check for nonsensical positions. (This happens in test cases which check code snippets.) +            if parameter_end_position > body_start_position: +                return + +            function_state.begin(function, function_name_start_position, body_start_position, end_position,                                   parameter_start_position, parameter_end_position, clean_lines)              return @@ -1471,7 +1497,7 @@ def check_for_function_lengths(clean_lines, line_number, function_state, error):      raw = clean_lines.raw_lines      raw_line = raw[line_number] -    if function_state.ending_line_number == line_number:  # last line +    if function_state.end_position.row == line_number:  # last line          if not search(r'\bNOLINT\b', raw_line):              function_state.check(error, line_number)      elif not match(r'^\s*$', line): @@ -1517,7 +1543,7 @@ def check_function_definition(clean_lines, line_number, function_state, error):         error: The function to call with any errors found.      """      # Only do checks when we have a function declaration. -    if line_number != function_state.body_start_line_number or not function_state.is_declaration: +    if line_number != function_state.body_start_position.row or not function_state.is_declaration:          return      parameter_list = function_state.parameter_list() @@ -1553,7 +1579,7 @@ def check_pass_ptr_usage(clean_lines, line_number, function_state, error):      lines = clean_lines.lines      line = lines[line_number] -    if line_number > function_state.body_start_line_number: +    if line_number > function_state.body_start_position.row:          matched_pass_ptr = match(r'^\s*Pass([A-Z][A-Za-z]*)Ptr<', line)          if matched_pass_ptr:              type_name = 'Pass%sPtr' % matched_pass_ptr.group(1) diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py index d39d2ba..868d3f6 100644 --- a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py +++ b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py @@ -342,6 +342,14 @@ class CppStyleTestBase(unittest.TestCase):                  'Blank line at the end of a code block.  Is this needed?'                  '  [whitespace/blank_line] [3]')) +    def assert_positions_equal(self, position, tuple_position): +        """Checks if the two positions are equal. + +        position: a cpp_style.Position object. +        tuple_position: a tuple (row, column) to compare against.""" +        self.assertEquals(position, cpp_style.Position(tuple_position[0], tuple_position[1]), +                          'position %s, tuple_position %s' % (position, tuple_position)) +  class FunctionDetectionTest(CppStyleTestBase):      def perform_function_detection(self, lines, function_information): @@ -354,9 +362,13 @@ class FunctionDetectionTest(CppStyleTestBase):              return          self.assertEquals(function_state.in_a_function, True)          self.assertEquals(function_state.current_function, function_information['name'] + '()') -        self.assertEquals(function_state.body_start_line_number, function_information['body_start_line_number']) -        self.assertEquals(function_state.ending_line_number, function_information['ending_line_number']) +        self.assertEquals(function_state.is_pure, function_information['is_pure'])          self.assertEquals(function_state.is_declaration, function_information['is_declaration']) +        self.assert_positions_equal(function_state.function_name_start_position, function_information['function_name_start_position']) +        self.assert_positions_equal(function_state.parameter_start_position, function_information['parameter_start_position']) +        self.assert_positions_equal(function_state.parameter_end_position, function_information['parameter_end_position']) +        self.assert_positions_equal(function_state.body_start_position, function_information['body_start_position']) +        self.assert_positions_equal(function_state.end_position, function_information['end_position'])          expected_parameters = function_information.get('parameter_list')          if expected_parameters:              actual_parameters = function_state.parameter_list() @@ -373,44 +385,105 @@ class FunctionDetectionTest(CppStyleTestBase):              ['void theTestFunctionName(int) {',               '}'],              {'name': 'theTestFunctionName', -             'body_start_line_number': 0, -             'ending_line_number': 1, +             'function_name_start_position': (0, 5), +             'parameter_start_position': (0, 24), +             'parameter_end_position': (0, 29), +             'body_start_position': (0, 30), +             'end_position': (1, 1), +             'is_pure': False,               'is_declaration': False})      def test_function_declaration_detection(self):          self.perform_function_detection(              ['void aFunctionName(int);'],              {'name': 'aFunctionName', -             'body_start_line_number': 0, -             'ending_line_number': 0, +             'function_name_start_position': (0, 5), +             'parameter_start_position': (0, 18), +             'parameter_end_position': (0, 23), +             'body_start_position': (0, 23), +             'end_position': (0, 24), +             'is_pure': False,               'is_declaration': True})          self.perform_function_detection(              ['CheckedInt<T> operator /(const CheckedInt<T> &lhs, const CheckedInt<T> &rhs);'],              {'name': 'operator /', -             'body_start_line_number': 0, -             'ending_line_number': 0, +             'function_name_start_position': (0, 14), +             'parameter_start_position': (0, 24), +             'parameter_end_position': (0, 76), +             'body_start_position': (0, 76), +             'end_position': (0, 77), +             'is_pure': False,               'is_declaration': True})          self.perform_function_detection(              ['CheckedInt<T> operator -(const CheckedInt<T> &lhs, const CheckedInt<T> &rhs);'],              {'name': 'operator -', -             'body_start_line_number': 0, -             'ending_line_number': 0, -            'is_declaration': True}) +             'function_name_start_position': (0, 14), +             'parameter_start_position': (0, 24), +             'parameter_end_position': (0, 76), +             'body_start_position': (0, 76), +             'end_position': (0, 77), +             'is_pure': False, +             'is_declaration': True})          self.perform_function_detection(              ['CheckedInt<T> operator !=(const CheckedInt<T> &lhs, const CheckedInt<T> &rhs);'],              {'name': 'operator !=', -             'body_start_line_number': 0, -             'ending_line_number': 0, +             'function_name_start_position': (0, 14), +             'parameter_start_position': (0, 25), +             'parameter_end_position': (0, 77), +             'body_start_position': (0, 77), +             'end_position': (0, 78), +             'is_pure': False,               'is_declaration': True})          self.perform_function_detection(              ['CheckedInt<T> operator +(const CheckedInt<T> &lhs, const CheckedInt<T> &rhs);'],              {'name': 'operator +', -             'body_start_line_number': 0, -             'ending_line_number': 0, +             'function_name_start_position': (0, 14), +             'parameter_start_position': (0, 24), +             'parameter_end_position': (0, 76), +             'body_start_position': (0, 76), +             'end_position': (0, 77), +             'is_pure': False, +             'is_declaration': True}) + +    def test_pure_function_detection(self): +        self.perform_function_detection( +            ['virtual void theTestFunctionName(int = 0);'], +            {'name': 'theTestFunctionName', +             'function_name_start_position': (0, 13), +             'parameter_start_position': (0, 32), +             'parameter_end_position': (0, 41), +             'body_start_position': (0, 41), +             'end_position': (0, 42), +             'is_pure': False, +             'is_declaration': True}) + +        self.perform_function_detection( +            ['virtual void theTestFunctionName(int) = 0;'], +            {'name': 'theTestFunctionName', +             'function_name_start_position': (0, 13), +             'parameter_start_position': (0, 32), +             'parameter_end_position': (0, 37), +             'body_start_position': (0, 41), +             'end_position': (0, 42), +             'is_pure': True, +             'is_declaration': True}) + +        # Hopefully, no one writes code like this but it is a tricky case. +        self.perform_function_detection( +            ['virtual void theTestFunctionName(int)', +             ' = ', +             ' 0 ;'], +            {'name': 'theTestFunctionName', +             'function_name_start_position': (0, 13), +             'parameter_start_position': (0, 32), +             'parameter_end_position': (0, 37), +             'body_start_position': (2, 3), +             'end_position': (2, 4), +             'is_pure': True,               'is_declaration': True})      def test_ignore_macros(self): @@ -425,8 +498,12 @@ class FunctionDetectionTest(CppStyleTestBase):              # This isn't a function but it looks like one to our simple              # algorithm and that is ok.              {'name': 'asm', -             'body_start_line_number': 2, -             'ending_line_number': 2, +             'function_name_start_position': (0, 0), +             'parameter_start_position': (0, 3), +             'parameter_end_position': (2, 1), +             'body_start_position': (2, 1), +             'end_position': (2, 2), +             'is_pure': False,               'is_declaration': True})          # Simple test case with something that is not a function. @@ -437,8 +514,12 @@ class FunctionDetectionTest(CppStyleTestBase):          function_state = self.perform_function_detection(              ['void functionName();'],              {'name': 'functionName', -             'body_start_line_number': 0, -             'ending_line_number': 0, +             'function_name_start_position': (0, 5), +             'parameter_start_position': (0, 17), +             'parameter_end_position': (0, 19), +             'body_start_position': (0, 19), +             'end_position': (0, 20), +             'is_pure': False,               'is_declaration': True,               'parameter_list': ()}) @@ -446,8 +527,12 @@ class FunctionDetectionTest(CppStyleTestBase):          function_state = self.perform_function_detection(              ['void functionName(int);'],              {'name': 'functionName', -             'body_start_line_number': 0, -             'ending_line_number': 0, +             'function_name_start_position': (0, 5), +             'parameter_start_position': (0, 17), +             'parameter_end_position': (0, 22), +             'body_start_position': (0, 22), +             'end_position': (0, 23), +             'is_pure': False,               'is_declaration': True,               'parameter_list':                   ({'type': 'int', 'name': '', 'row': 0},)}) @@ -456,8 +541,12 @@ class FunctionDetectionTest(CppStyleTestBase):          function_state = self.perform_function_detection(              ['void functionName(unsigned a, short b, long c, long long short unsigned int);'],              {'name': 'functionName', -             'body_start_line_number': 0, -             'ending_line_number': 0, +             'function_name_start_position': (0, 5), +             'parameter_start_position': (0, 17), +             'parameter_end_position': (0, 76), +             'body_start_position': (0, 76), +             'end_position': (0, 77), +             'is_pure': False,               'is_declaration': True,               'parameter_list':                   ({'type': 'unsigned', 'name': 'a', 'row': 0}, @@ -469,8 +558,12 @@ class FunctionDetectionTest(CppStyleTestBase):          function_state = self.perform_function_detection(              ['virtual void determineARIADropEffects(Vector<String>*&, const unsigned long int*&, const MediaPlayer::Preload, Other<Other2, Other3<P1, P2> >, int);'],              {'name': 'determineARIADropEffects', -             'body_start_line_number': 0, -             'ending_line_number': 0, +             'parameter_start_position': (0, 37), +             'function_name_start_position': (0, 13), +             'parameter_end_position': (0, 147), +             'body_start_position': (0, 147), +             'end_position': (0, 148), +             'is_pure': False,               'is_declaration': True,               'parameter_list':                   ({'type': 'Vector<String>*&', 'name': '', 'row': 0}, @@ -486,8 +579,12 @@ class FunctionDetectionTest(CppStyleTestBase):               'const ComplexTemplate<Class1, NestedTemplate<P1, P2> >* const * param = new ComplexTemplate<Class1, NestedTemplate<P1, P2> >(34, 42),',               'int* myCount = 0);'],              {'name': 'aFunctionName', -             'body_start_line_number': 3, -             'ending_line_number': 3, +             'function_name_start_position': (0, 32), +             'parameter_start_position': (0, 45), +             'parameter_end_position': (3, 17), +             'body_start_position': (3, 17), +             'end_position': (3, 18), +             'is_pure': False,               'is_declaration': True,               'parameter_list':                   ({'type': 'PassRefPtr<MyClass>', 'name': 'paramName', 'row': 0}, @@ -523,6 +620,24 @@ class CppStyleTest(CppStyleTestBase):          cpp_style.remove_multi_line_comments_from_range(lines, 1, 4)          self.assertEquals(['a', '// dummy', '// dummy', '// dummy', 'b'], lines) +    def test_position(self): +        position = cpp_style.Position(3, 4) +        self.assert_positions_equal(position, (3, 4)) +        self.assertEquals(position.row, 3) +        self.assertTrue(position > cpp_style.Position(position.row - 1, position.column + 1)) +        self.assertTrue(position > cpp_style.Position(position.row, position.column - 1)) +        self.assertTrue(position < cpp_style.Position(position.row, position.column + 1)) +        self.assertTrue(position < cpp_style.Position(position.row + 1, position.column - 1)) +        self.assertEquals(position.__str__(), '(3, 4)') + +    def test_close_expression(self): +        self.assertEquals(cpp_style.Position(1, -1), cpp_style.close_expression([')('], cpp_style.Position(0, 1))) +        self.assertEquals(cpp_style.Position(1, -1), cpp_style.close_expression([') ()'], cpp_style.Position(0, 1))) +        self.assertEquals(cpp_style.Position(0, 4), cpp_style.close_expression([')[)]'], cpp_style.Position(0, 1))) +        self.assertEquals(cpp_style.Position(0, 5), cpp_style.close_expression(['}{}{}'], cpp_style.Position(0, 3))) +        self.assertEquals(cpp_style.Position(1, 1), cpp_style.close_expression(['}{}{', '}'], cpp_style.Position(0, 3))) +        self.assertEquals(cpp_style.Position(2, -1), cpp_style.close_expression(['][][', ' '], cpp_style.Position(0, 3))) +      def test_spaces_at_end_of_line(self):          self.assert_lint(              '// Hello there ', diff --git a/Tools/Scripts/webkitpy/style_references.py b/Tools/Scripts/webkitpy/style_references.py index a21e931..c92bad9 100644 --- a/Tools/Scripts/webkitpy/style_references.py +++ b/Tools/Scripts/webkitpy/style_references.py @@ -70,5 +70,4 @@ class WebKitCheckout(object):          return self._scm.checkout_root      def create_patch(self, git_commit, changed_files=None): -        # FIXME: SCM.create_patch should understand how to handle None. -        return self._scm.create_patch(git_commit, changed_files=changed_files or []) +        return self._scm.create_patch(git_commit, changed_files=changed_files) diff --git a/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py b/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py index 4bdc79b..3be2556 100644 --- a/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py +++ b/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py @@ -46,7 +46,11 @@ class CommitQueueTaskDelegate(object):      def layout_test_results(self):          raise NotImplementedError("subclasses must implement") -    def report_flaky_tests(self, patch, flaky_tests): +    def archive_last_layout_test_results(self, patch): +        raise NotImplementedError("subclasses must implement") + +    # We could make results_archive optional, but for now it's required. +    def report_flaky_tests(self, patch, flaky_tests, results_archive):          raise NotImplementedError("subclasses must implement") @@ -66,6 +70,8 @@ class CommitQueueTask(object):              return False          if not self._patch.committer():              return False +        if not self._patch.review() != "-": +            return False          # Reviewer is not required. Missing reviewers will be caught during          # the ChangeLog check during landing.          return True @@ -168,8 +174,8 @@ class CommitQueueTask(object):          "Landed patch",          "Unable to land patch") -    def _report_flaky_tests(self, flaky_test_results): -        self._delegate.report_flaky_tests(self._patch, flaky_test_results) +    def _report_flaky_tests(self, flaky_test_results, results_archive): +        self._delegate.report_flaky_tests(self._patch, flaky_test_results, results_archive)      def _test_patch(self):          if self._patch.is_rollout(): @@ -177,14 +183,15 @@ class CommitQueueTask(object):          if self._test():              return True -        first_failing_results = self._failing_results_from_last_run() -        first_failing_tests = [result.filename for result in first_failing_results] +        first_results = self._failing_results_from_last_run() +        first_failing_tests = [result.filename for result in first_results] +        first_results_archive = self._delegate.archive_last_layout_test_results(self._patch)          if self._test(): -            self._report_flaky_tests(first_failing_results) +            self._report_flaky_tests(first_results, first_results_archive)              return True -        second_failing_results = self._failing_results_from_last_run() -        second_failing_tests = [result.filename for result in second_failing_results] +        second_results = self._failing_results_from_last_run() +        second_failing_tests = [result.filename for result in second_results]          if first_failing_tests != second_failing_tests:              # We could report flaky tests here, but since run-webkit-tests              # is run with --exit-after-N-failures=1, we would need to diff --git a/Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py b/Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py index f279cac..26231ae 100644 --- a/Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py +++ b/Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py @@ -29,6 +29,7 @@  from datetime import datetime  import unittest +from webkitpy.common.net import bugzilla  from webkitpy.common.system.deprecated_logging import error, log  from webkitpy.common.system.outputcapture import OutputCapture  from webkitpy.layout_tests.layout_package import test_results @@ -64,9 +65,15 @@ class MockCommitQueue(CommitQueueTaskDelegate):      def layout_test_results(self):          return None -    def report_flaky_tests(self, patch, flaky_results): +    def report_flaky_tests(self, patch, flaky_results, results_archive):          flaky_tests = [result.filename for result in flaky_results] -        log("report_flaky_tests: patch='%s' flaky_tests='%s'" % (patch.id(), flaky_tests)) +        log("report_flaky_tests: patch='%s' flaky_tests='%s' archive='%s'" % (patch.id(), flaky_tests, results_archive.filename)) + +    def archive_last_layout_test_results(self, patch): +        log("archive_last_layout_test_results: patch='%s'" % patch.id()) +        archive = Mock() +        archive.filename = "mock-archive-%s.zip" % patch.id() +        return archive  class CommitQueueTaskTest(unittest.TestCase): @@ -193,9 +200,10 @@ run_webkit_patch: ['build', '--no-clean', '--no-update', '--build-style=both']  command_passed: success_message='Built patch' patch='197'  run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']  command_failed: failure_message='Patch does not pass tests' script_error='MOCK tests failure' patch='197' +archive_last_layout_test_results: patch='197'  run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']  command_passed: success_message='Passed tests' patch='197' -report_flaky_tests: patch='197' flaky_tests='[]' +report_flaky_tests: patch='197' flaky_tests='[]' archive='mock-archive-197.zip'  run_webkit_patch: ['land-attachment', '--force-clean', '--ignore-builders', '--non-interactive', '--parent-command=commit-queue', 197]  command_passed: success_message='Landed patch' patch='197'  """ @@ -225,6 +233,7 @@ run_webkit_patch: ['build', '--no-clean', '--no-update', '--build-style=both']  command_passed: success_message='Built patch' patch='197'  run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']  command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure' patch='197' +archive_last_layout_test_results: patch='197'  run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']  command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure again' patch='197'  """ @@ -262,6 +271,7 @@ run_webkit_patch: ['build', '--no-clean', '--no-update', '--build-style=both']  command_passed: success_message='Built patch' patch='197'  run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']  command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure' patch='197' +archive_last_layout_test_results: patch='197'  run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']  command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure again' patch='197'  run_webkit_patch: ['build-and-test', '--force-clean', '--no-update', '--build', '--test', '--non-interactive'] @@ -289,6 +299,7 @@ run_webkit_patch: ['build', '--no-clean', '--no-update', '--build-style=both']  command_passed: success_message='Built patch' patch='197'  run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']  command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure' patch='197' +archive_last_layout_test_results: patch='197'  run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']  command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure again' patch='197'  run_webkit_patch: ['build-and-test', '--force-clean', '--no-update', '--build', '--test', '--non-interactive'] @@ -320,3 +331,24 @@ command_failed: failure_message='Unable to land patch' script_error='MOCK land f  """          # FIXME: This should really be expect_retry=True for a better user experiance.          self._run_through_task(commit_queue, expected_stderr, ScriptError) + +    def _expect_validate(self, patch, is_valid): +        class MockDelegate(object): +            def refetch_patch(self, patch): +                return patch + +        task = CommitQueueTask(MockDelegate(), patch) +        self.assertEquals(task._validate(), is_valid) + +    def _mock_patch(self, attachment_dict={}, bug_dict={'bug_status': 'NEW'}, committer="fake"): +        bug = bugzilla.Bug(bug_dict, None) +        patch = bugzilla.Attachment(attachment_dict, bug) +        patch._committer = committer +        return patch + +    def test_validate(self): +        self._expect_validate(self._mock_patch(), True) +        self._expect_validate(self._mock_patch({'is_obsolete': True}), False) +        self._expect_validate(self._mock_patch(bug_dict={'bug_status': 'CLOSED'}), False) +        self._expect_validate(self._mock_patch(committer=None), False) +        self._expect_validate(self._mock_patch({'review': '-'}), False) diff --git a/Tools/Scripts/webkitpy/tool/bot/feeders.py b/Tools/Scripts/webkitpy/tool/bot/feeders.py index 046c4c1..0b7f23d 100644 --- a/Tools/Scripts/webkitpy/tool/bot/feeders.py +++ b/Tools/Scripts/webkitpy/tool/bot/feeders.py @@ -54,6 +54,7 @@ class CommitQueueFeeder(AbstractFeeder):      def feed(self):          patches = self._validate_patches() +        patches = self._patches_with_acceptable_review_flag(patches)          patches = sorted(patches, self._patch_cmp)          patch_ids = [patch.id() for patch in patches]          self._update_work_items(patch_ids) @@ -61,6 +62,10 @@ class CommitQueueFeeder(AbstractFeeder):      def _patches_for_bug(self, bug_id):          return self._tool.bugs.fetch_bug(bug_id).commit_queued_patches(include_invalid=True) +    # Filters out patches with r? or r-, only r+ or no review are OK to land. +    def _patches_with_acceptable_review_flag(self, patches): +        return [patch for patch in patches if patch.review() in [None, '+']] +      def _validate_patches(self):          # Not using BugzillaQueries.fetch_patches_from_commit_queue() so we can reject patches with invalid committers/reviewers.          bug_ids = self._tool.bugs.queries.fetch_bug_ids_from_commit_queue() diff --git a/Tools/Scripts/webkitpy/tool/bot/feeders_unittest.py b/Tools/Scripts/webkitpy/tool/bot/feeders_unittest.py index 580f840..e956a8f 100644 --- a/Tools/Scripts/webkitpy/tool/bot/feeders_unittest.py +++ b/Tools/Scripts/webkitpy/tool/bot/feeders_unittest.py @@ -68,3 +68,13 @@ Feeding commit-queue items [106, 197]          queue = CommitQueueFeeder(MockTool())          attachments.sort(queue._patch_cmp)          self.assertEqual(attachments, expected_sort) + +    def test_patches_with_acceptable_review_flag(self): +        class MockPatch(object): +            def __init__(self, patch_id, review): +                self.id = patch_id +                self.review = lambda: review + +        feeder = CommitQueueFeeder(MockTool()) +        patches = [MockPatch(1, None), MockPatch(2, '-'), MockPatch(3, "+")] +        self.assertEquals([patch.id for patch in feeder._patches_with_acceptable_review_flag(patches)], [1, 3]) diff --git a/Tools/Scripts/webkitpy/tool/bot/flakytestreporter.py b/Tools/Scripts/webkitpy/tool/bot/flakytestreporter.py index 91fcb85..270a656 100644 --- a/Tools/Scripts/webkitpy/tool/bot/flakytestreporter.py +++ b/Tools/Scripts/webkitpy/tool/bot/flakytestreporter.py @@ -131,13 +131,10 @@ If you would like to track this test fix with another bug, please close this bug          flake_message = "The %s just saw %s flake (%s) while processing attachment %s on bug %s." % (self._bot_name, flaky_result.filename, ", ".join(failure_messages), patch.id(), patch.bug_id())          return "%s\n%s" % (flake_message, self._bot_information()) -    def _results_diff_path_for_test(self, flaky_test): +    def _results_diff_path_for_test(self, test_path):          # FIXME: This is a big hack.  We should get this path from results.json          # except that old-run-webkit-tests doesn't produce a results.json          # so we just guess at the file path. -        results_path = self._tool.port().layout_tests_results_path() -        results_directory = os.path.dirname(results_path) -        test_path = os.path.join(results_directory, flaky_test)          (test_path_root, _) = os.path.splitext(test_path)          return "%s-diffs.txt" % test_path_root @@ -153,7 +150,32 @@ If you would like to track this test fix with another bug, please close this bug          else:              self._tool.bugs.post_comment_to_bug(bug.id(), latest_flake_message) -    def report_flaky_tests(self, flaky_test_results, patch): +    # This method is needed because our archive paths include a leading tmp/layout-test-results +    def _find_in_archive(self, path, archive): +        for archived_path in archive.namelist(): +            # Archives are currently created with full paths. +            if archived_path.endswith(path): +                return archived_path +        return None + +    def _attach_failure_diff(self, flake_bug_id, flaky_test, results_archive): +        results_diff_path = self._results_diff_path_for_test(flaky_test) +        # Check to make sure that the path makes sense. +        # Since we're not actually getting this path from the results.html +        # there is a chance it's wrong. +        bot_id = self._tool.status_server.bot_id or "bot" +        archive_path = self._find_in_archive(results_diff_path, results_archive) +        if archive_path: +            results_diff = results_archive.read(archive_path) +            description = "Failure diff from %s" % bot_id +            self._tool.bugs.add_attachment_to_bug(flake_bug_id, results_diff, description, filename="failure.diff") +        else: +            _log.warn("%s does not exist in results archive, uploading entire archive." % results_diff_path) +            description = "Archive of layout-test-results from %s" % bot_id +            # results_archive is a ZipFile object, grab the File object (.fp) to pass to Mechanize for uploading. +            self._tool.bugs.add_attachment_to_bug(flake_bug_id, results_archive.fp, description, filename="layout-test-results.zip") + +    def report_flaky_tests(self, patch, flaky_test_results, results_archive):          message = "The %s encountered the following flaky tests while processing attachment %s:\n\n" % (self._bot_name, patch.id())          for flaky_result in flaky_test_results:              flaky_test = flaky_result.filename @@ -165,20 +187,12 @@ If you would like to track this test fix with another bug, please close this bug                  flake_bug_id = self._create_bug_for_flaky_test(flaky_test, author_emails, latest_flake_message)              else:                  bug = self._follow_duplicate_chain(bug) +                # FIXME: Ideally we'd only make one comment per flake, not two.  But that's not possible +                # in all cases (e.g. when reopening), so for now file attachment and comment are separate.                  self._update_bug_for_flaky_test(bug, latest_flake_message)                  flake_bug_id = bug.id() -            # FIXME: Ideally we'd only make one comment per flake, not two.  But that's not possible -            # in all cases (e.g. when reopening), so for now we do the attachment in a second step. -            results_diff_path = self._results_diff_path_for_test(flaky_test) -            # Check to make sure that the path makes sense. -            # Since we're not actually getting this path from the results.html -            # there is a high probaility it's totally wrong. -            if self._tool.filesystem.exists(results_diff_path): -                results_diff = self._tool.filesystem.read_binary_file(results_diff_path) -                bot_id = self._tool.status_server.bot_id or "bot" -                self._tool.bugs.add_attachment_to_bug(flake_bug_id, results_diff, "Failure diff from %s" % bot_id, filename="failure.diff") -            else: -                _log.error("%s does not exist as expected, not uploading." % results_diff_path) + +            self._attach_failure_diff(flake_bug_id, flaky_test, results_archive)              message += "%s bug %s%s\n" % (flaky_test, flake_bug_id, self._optional_author_string(author_emails))          message += "The %s is continuing to process your patch." % self._bot_name diff --git a/Tools/Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py b/Tools/Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py index 631f8d1..26c98c1 100644 --- a/Tools/Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py +++ b/Tools/Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py @@ -33,6 +33,7 @@ from webkitpy.common.system.filesystem_mock import MockFileSystem  from webkitpy.common.system.outputcapture import OutputCapture  from webkitpy.layout_tests.layout_package import test_results  from webkitpy.layout_tests.layout_package import test_failures +from webkitpy.thirdparty.mock import Mock  from webkitpy.tool.bot.flakytestreporter import FlakyTestReporter  from webkitpy.tool.mocktool import MockTool, MockStatusServer @@ -140,7 +141,15 @@ The dummy-queue is continuing to process your patch.  """          test_results = [self._mock_test_result('foo/bar.html')] -        OutputCapture().assert_outputs(self, reporter.report_flaky_tests, [test_results, patch], expected_stderr=expected_stderr) + +        class MockZipFile(object): +            def read(self, path): +                return "" + +            def namelist(self): +                return ['foo/bar-diffs.txt'] + +        OutputCapture().assert_outputs(self, reporter.report_flaky_tests, [patch, test_results, MockZipFile()], expected_stderr=expected_stderr)      def test_optional_author_string(self):          reporter = FlakyTestReporter(MockTool(), 'dummy-queue') @@ -150,6 +159,15 @@ The dummy-queue is continuing to process your patch.      def test_results_diff_path_for_test(self):          reporter = FlakyTestReporter(MockTool(), 'dummy-queue') -        self.assertEqual(reporter._results_diff_path_for_test("test.html"), "/mock/test-diffs.txt") +        self.assertEqual(reporter._results_diff_path_for_test("test.html"), "test-diffs.txt") + +    def test_find_in_archive(self): +        reporter = FlakyTestReporter(MockTool(), 'dummy-queue') + +        class MockZipFile(object): +            def namelist(self): +                return ["tmp/layout-test-results/foo/bar-diffs.txt"] -    # report_flaky_tests is also tested by queues_unittest +        reporter._find_in_archive("foo/bar-diffs.txt", MockZipFile()) +        # This is not ideal, but its +        reporter._find_in_archive("txt", MockZipFile()) diff --git a/Tools/Scripts/webkitpy/tool/commands/queues.py b/Tools/Scripts/webkitpy/tool/commands/queues.py index 5628543..42321cf 100644 --- a/Tools/Scripts/webkitpy/tool/commands/queues.py +++ b/Tools/Scripts/webkitpy/tool/commands/queues.py @@ -309,12 +309,32 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler, CommitQueueTaskD              return None          return LayoutTestResults.results_from_string(results_html) +    def _results_directory(self): +        results_path = self._tool.port().layout_tests_results_path() +        # FIXME: This is wrong in two ways: +        # 1. It assumes that results.html is at the top level of the results tree. +        # 2. This uses the "old" ports.py infrastructure instead of the new layout_tests/port +        # which will not support Chromium.  However the new arch doesn't work with old-run-webkit-tests +        # so we have to use this for now. +        return os.path.dirname(results_path) + +    def archive_last_layout_test_results(self, patch): +        results_directory = self._results_directory() +        results_name, _ = os.path.splitext(os.path.basename(results_directory)) +        # Note: We name the zip with the bug_id instead of patch_id to match work_item_log_path(). +        zip_path = self._tool.workspace.find_unused_filename(self._log_directory(), "%s-%s" % (patch.bug_id(), results_name), "zip") +        archive = self._tool.workspace.create_zip(zip_path, results_directory) +        # Remove the results directory to prevent http logs, etc. from getting huge between runs. +        # We could have create_zip remove the original, but this is more explicit. +        self._tool.filesystem.remove_tree(results_directory, ignore_errors=True) +        return archive +      def refetch_patch(self, patch):          return self._tool.bugs.fetch_attachment(patch.id()) -    def report_flaky_tests(self, patch, flaky_test_results): +    def report_flaky_tests(self, patch, flaky_test_results, results_archive=None):          reporter = FlakyTestReporter(self._tool, self.name) -        reporter.report_flaky_tests(flaky_test_results, patch) +        reporter.report_flaky_tests(patch, flaky_test_results, results_archive)      # StepSequenceErrorHandler methods diff --git a/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py b/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py index 34a6a64..8f5c9e6 100644 --- a/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py +++ b/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py @@ -27,9 +27,11 @@  # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.  import os +import StringIO  from webkitpy.common.checkout.scm import CheckoutNeedsUpdate  from webkitpy.common.net.bugzilla import Attachment +from webkitpy.common.system.filesystem_mock import MockFileSystem  from webkitpy.common.system.outputcapture import OutputCapture  from webkitpy.layout_tests.layout_package import test_results  from webkitpy.layout_tests.layout_package import test_failures @@ -342,12 +344,14 @@ The commit-queue just saw foo/bar.html flake (Text diff mismatch) while processi  Port: MockPort  Platform: MockPlatform 1.0  --- End comment --- +MOCK add_attachment_to_bug: bug_id=76, description=Failure diff from bot filename=failure.diff  MOCK bug comment: bug_id=76, cc=None  --- Begin comment ---  The commit-queue just saw bar/baz.html flake (Text diff mismatch) while processing attachment 197 on bug 42.  Port: MockPort  Platform: MockPlatform 1.0  --- End comment --- +MOCK add_attachment_to_bug: bug_id=76, description=Archive of layout-test-results from bot filename=layout-test-results.zip  MOCK bug comment: bug_id=42, cc=None  --- Begin comment ---  The commit-queue encountered the following flaky tests while processing attachment 197: @@ -360,7 +364,19 @@ The commit-queue is continuing to process your patch.  """          test_names = ["foo/bar.html", "bar/baz.html"]          test_results = [self._mock_test_result(name) for name in test_names] -        OutputCapture().assert_outputs(self, queue.report_flaky_tests, [QueuesTest.mock_work_item, test_results], expected_stderr=expected_stderr) + +        class MockZipFile(object): +            def __init__(self): +                self.fp = StringIO() + +            def read(self, path): +                return "" + +            def namelist(self): +                # This is intentionally missing one diffs.txt to exercise the "upload the whole zip" codepath. +                return ['foo/bar-diffs.txt'] + +        OutputCapture().assert_outputs(self, queue.report_flaky_tests, [QueuesTest.mock_work_item, test_results, MockZipFile()], expected_stderr=expected_stderr)      def test_layout_test_results(self):          queue = CommitQueue() @@ -370,6 +386,12 @@ The commit-queue is continuing to process your patch.          queue._read_file_contents = lambda path: ""          self.assertEquals(queue.layout_test_results(), None) +    def test_archive_last_layout_test_results(self): +        queue = CommitQueue() +        queue.bind_to_tool(MockTool()) +        patch = queue._tool.bugs.fetch_attachment(128) +        queue.archive_last_layout_test_results(patch) +  class StyleQueueTest(QueuesTest):      def test_style_queue(self): diff --git a/Tools/Scripts/webkitpy/tool/main.py b/Tools/Scripts/webkitpy/tool/main.py index 0006e87..76d5bef 100755 --- a/Tools/Scripts/webkitpy/tool/main.py +++ b/Tools/Scripts/webkitpy/tool/main.py @@ -40,10 +40,7 @@ from webkitpy.common.net.bugzilla import Bugzilla  from webkitpy.common.net.buildbot import BuildBot  from webkitpy.common.net.irc.ircproxy import IRCProxy  from webkitpy.common.net.statusserver import StatusServer -from webkitpy.common.system.executive import Executive -from webkitpy.common.system.filesystem import FileSystem -from webkitpy.common.system.platforminfo import PlatformInfo -from webkitpy.common.system.user import User +from webkitpy.common.system import executive, filesystem, platforminfo, user, workspace  from webkitpy.layout_tests import port  from webkitpy.tool.multicommandtool import MultiCommandTool  import webkitpy.tool.commands as commands @@ -52,6 +49,7 @@ import webkitpy.tool.commands as commands  class WebKitPatch(MultiCommandTool):      global_options = [          make_option("-v", "--verbose", action="store_true", dest="verbose", default=False, help="enable all logging"), +        make_option("-d", "--directory", action="append", dest="patch_directories", default=[], help="Directory to look at for changed files"),          make_option("--dry-run", action="store_true", dest="dry_run", default=False, help="do not touch remote servers"),          make_option("--status-host", action="store", dest="status_host", type="string", help="Hostname (e.g. localhost or commit.webkit.org) where status updates should be posted."),          make_option("--bot-id", action="store", dest="bot_id", type="string", help="Identifier for this bot (if multiple bots are running for a queue)"), @@ -70,21 +68,22 @@ class WebKitPatch(MultiCommandTool):          # manual getter functions (e.g. scm()).          self.bugs = Bugzilla()          self.buildbot = BuildBot() -        self.executive = Executive() +        self.executive = executive.Executive()          self._irc = None -        self.filesystem = FileSystem() +        self.filesystem = filesystem.FileSystem() +        self.workspace = workspace.Workspace(self.filesystem, self.executive)          self._port = None -        self.user = User() +        self.user = user.User()          self._scm = None          self._checkout = None          self.status_server = StatusServer()          self.port_factory = port.factory -        self.platform = PlatformInfo() +        self.platform = platforminfo.PlatformInfo()      def scm(self):          # Lazily initialize SCM to not error-out before command line parsing (or when running non-scm commands).          if not self._scm: -            self._scm = default_scm() +            self._scm = default_scm(self._options.patch_directories)          return self._scm      def checkout(self): diff --git a/Tools/Scripts/webkitpy/tool/mocktool.py b/Tools/Scripts/webkitpy/tool/mocktool.py index eb7c248..7db2996 100644 --- a/Tools/Scripts/webkitpy/tool/mocktool.py +++ b/Tools/Scripts/webkitpy/tool/mocktool.py @@ -623,10 +623,10 @@ class MockStatusServer(object):  # FIXME: Unify with common.system.executive_mock.MockExecutive.  class MockExecutive(Mock):      def __init__(self, should_log): -        self._should_log = should_log +        self.should_log = should_log      def run_and_throw_if_fail(self, args, quiet=False): -        if self._should_log: +        if self.should_log:              log("MOCK run_and_throw_if_fail: %s" % args)          return "MOCK output of child process" @@ -638,7 +638,7 @@ class MockExecutive(Mock):                      return_exit_code=False,                      return_stderr=True,                      decode_output=False): -        if self._should_log: +        if self.should_log:              log("MOCK run_command: %s" % args)          return "MOCK output of child process" @@ -686,6 +686,14 @@ class MockPlatformInfo(object):          return "MockPlatform 1.0" +class MockWorkspace(object): +    def find_unused_filename(self, directory, name, extension, search_limit=10): +        return "%s/%s.%s" % (directory, name, extension) + +    def create_zip(self, zip_path, source_path): +        pass + +  class MockTool(object):      def __init__(self, log_executive=False): @@ -694,6 +702,7 @@ class MockTool(object):          self.buildbot = MockBuildBot()          self.executive = MockExecutive(should_log=log_executive)          self.filesystem = MockFileSystem() +        self.workspace = MockWorkspace()          self._irc = None          self.user = MockUser()          self._scm = MockSCM() diff --git a/Tools/Scripts/webkitpy/tool/steps/preparechangelog.py b/Tools/Scripts/webkitpy/tool/steps/preparechangelog.py index 099dfe3..392cd32 100644 --- a/Tools/Scripts/webkitpy/tool/steps/preparechangelog.py +++ b/Tools/Scripts/webkitpy/tool/steps/preparechangelog.py @@ -70,6 +70,8 @@ class PrepareChangeLog(AbstractStep):          if self._tool.scm().supports_local_commits():              args.append("--merge-base=%s" % self._tool.scm().merge_base(self._options.git_commit)) +        args.extend(self._changed_files(state)) +          try:              self._tool.executive.run_and_throw_if_fail(args, self._options.quiet)          except ScriptError, e: diff --git a/Tools/Scripts/webkitpy/tool/steps/validatechangelogs.py b/Tools/Scripts/webkitpy/tool/steps/validatechangelogs.py index e812f94..a27ed77 100644 --- a/Tools/Scripts/webkitpy/tool/steps/validatechangelogs.py +++ b/Tools/Scripts/webkitpy/tool/steps/validatechangelogs.py @@ -44,6 +44,9 @@ class ValidateChangeLogs(AbstractStep):          # later than that, assume that the entry is wrong.          if diff_file.lines[0][0] < 8:              return True +        if self._options.non_interactive: +            return False +          log("The diff to %s looks wrong.  Are you sure your ChangeLog entry is at the top of the file?" % (diff_file.filename))          # FIXME: Do we need to make the file path absolute?          self._tool.scm().diff_for_file(diff_file.filename) diff --git a/Tools/Scripts/webkitpy/tool/steps/validatechangelogs_unittest.py b/Tools/Scripts/webkitpy/tool/steps/validatechangelogs_unittest.py index 66db793..db35a58 100644 --- a/Tools/Scripts/webkitpy/tool/steps/validatechangelogs_unittest.py +++ b/Tools/Scripts/webkitpy/tool/steps/validatechangelogs_unittest.py @@ -36,20 +36,24 @@ from webkitpy.tool.steps.validatechangelogs import ValidateChangeLogs  class ValidateChangeLogsTest(unittest.TestCase): -    def _assert_start_line_produces_output(self, start_line, should_prompt_user=False): +    def _assert_start_line_produces_output(self, start_line, should_fail=False, non_interactive=False):          tool = MockTool()          tool._checkout.is_path_to_changelog = lambda path: True -        step = ValidateChangeLogs(tool, MockOptions(git_commit=None)) +        step = ValidateChangeLogs(tool, MockOptions(git_commit=None, non_interactive=non_interactive))          diff_file = Mock()          diff_file.filename = "mock/ChangeLog"          diff_file.lines = [(start_line, start_line, "foo")]          expected_stdout = expected_stderr = "" -        if should_prompt_user: +        if should_fail and not non_interactive:              expected_stdout = "OK to continue?\n"              expected_stderr = "The diff to mock/ChangeLog looks wrong.  Are you sure your ChangeLog entry is at the top of the file?\n" -        OutputCapture().assert_outputs(self, step._check_changelog_diff, [diff_file], expected_stdout=expected_stdout, expected_stderr=expected_stderr) +        result = OutputCapture().assert_outputs(self, step._check_changelog_diff, [diff_file], expected_stdout=expected_stdout, expected_stderr=expected_stderr) +        self.assertEqual(not result, should_fail)      def test_check_changelog_diff(self): -        self._assert_start_line_produces_output(1, should_prompt_user=False) -        self._assert_start_line_produces_output(7, should_prompt_user=False) -        self._assert_start_line_produces_output(8, should_prompt_user=True) +        self._assert_start_line_produces_output(1) +        self._assert_start_line_produces_output(7) +        self._assert_start_line_produces_output(8, should_fail=True) + +        self._assert_start_line_produces_output(1, non_interactive=False) +        self._assert_start_line_produces_output(8, non_interactive=True, should_fail=True)  | 
