2010-05-02 Chris Jerdonek <cjerdonek@webkit.org>
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 3 May 2010 01:49:27 +0000 (01:49 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 3 May 2010 01:49:27 +0000 (01:49 +0000)
        Reviewed by Eric Seidel.

        https://bugs.webkit.org/show_bug.cgi?id=38319

        * Scripts/VCSUtils.pm:
          - In parseDiffHeader()--
            - Added an "scmFormat" hash key to the return value to represent
              whether the diff is Git or SVN formatted.
            - Adjusted the code so the value of "copiedFromPath" will
              be undef rather than "does not exist" if the file was not
              copied.

        * Scripts/webkitperl/VCSUtils_unittest/parseDiffHeader.pl:
          - Added a FIXME to refactor these unit tests to use is_deeply().

        * Scripts/webkitperl/VCSUtils_unittest/parseDiffHeader.pl:
          - Updated the unit tests to test the "scmFormat" value.
          - Simplified the unit tests by refactoring them to use is_deeply().

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@58663 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKitTools/ChangeLog
WebKitTools/Scripts/VCSUtils.pm
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiffHeader.pl

index e76aa085b2ba5f2fc12c565f3caf6cfdfbc6750c..40b97b09a0163d15346547a6c0245bbe4d1e2631 100644 (file)
@@ -1,3 +1,24 @@
+2010-05-02  Chris Jerdonek  <cjerdonek@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        https://bugs.webkit.org/show_bug.cgi?id=38319
+
+        * Scripts/VCSUtils.pm:
+          - In parseDiffHeader()--
+            - Added an "scmFormat" hash key to the return value to represent
+              whether the diff is Git or SVN formatted.
+            - Adjusted the code so the value of "copiedFromPath" will
+              be undef rather than "does not exist" if the file was not
+              copied.
+
+        * Scripts/webkitperl/VCSUtils_unittest/parseDiffHeader.pl:
+          - Added a FIXME to refactor these unit tests to use is_deeply().
+
+        * Scripts/webkitperl/VCSUtils_unittest/parseDiffHeader.pl:
+          - Updated the unit tests to test the "scmFormat" value.
+          - Simplified the unit tests by refactoring them to use is_deeply().
+
 2010-05-01  Daniel Bates  <dbates@rim.com>
 
         Reviewed by Chris Jerdonek.
index dff1c4d5e926b35a93310d5c8f524358942fc836..ed810524cce5ffcc1ef758b3e3ae09c696ef3025 100644 (file)
@@ -439,6 +439,7 @@ sub gitdiff2svndiff($)
 #     copiedFromPath: if a file copy, the path from which the file was
 #                     copied. Otherwise, undefined.
 #     indexPath: the path in the "Index:" line.
+#     scmFormat: The string "git" or "svn" depending on the format.
 #     sourceRevision: the revision number of the source. This is the same
 #                     as the revision number the file was copied from, in
 #                     the case of a file copy.
@@ -465,6 +466,7 @@ sub parseDiffHeader($$)
 
     my %header;
 
+    my $copiedFromPath;
     my $foundHeaderEnding;
     my $lastReadLine; 
     my $sourceRevision;
@@ -486,7 +488,7 @@ sub parseDiffHeader($$)
                 if (/\(from (\S+):(\d+)\)$/) {
                     # The "from" clause is created by svn-create-patch, in
                     # which case there is always also a "revision" clause.
-                    $header{copiedFromPath} = $1;
+                    $copiedFromPath = $1;
                     die("Revision number \"$2\" in \"from\" clause does not match " .
                         "source revision number \"$sourceRevision\".") if ($2 != $sourceRevision);
                 }
@@ -509,7 +511,9 @@ sub parseDiffHeader($$)
         die("Did not find end of header block corresponding to index path \"$indexPath\".");
     }
 
+    $header{copiedFromPath} = $copiedFromPath;
     $header{indexPath} = $indexPath;
+    $header{scmFormat} = $filter ? "git" : "svn";
     $header{sourceRevision} = $sourceRevision;
     $header{svnConvertedText} = $svnConvertedText;
 
@@ -586,6 +590,7 @@ sub parseDiff($$)
     my %diffHashRef;
     $diffHashRef{copiedFromPath} = $headerHashRef->{copiedFromPath};
     $diffHashRef{indexPath} = $headerHashRef->{indexPath};
+    # FIXME: Also add scmFormat from the $headerHashRef.
     $diffHashRef{sourceRevision} = $headerHashRef->{sourceRevision};
     $diffHashRef{svnConvertedText} = $svnText;
 
index e553d9669780e844da89e01ff0381051a3e44b5a..ee68b5e4477c99f6f54dc83f44a6a3947f343bf9 100644 (file)
@@ -30,6 +30,9 @@ use warnings;
 use Test::More;
 use VCSUtils;
 
+# FIXME: Refactor the unit tests in this file to use is_deeply().
+#        See the unit tests for parseDiffHeader() as an example.
+
 my @diffHashRefKeys = ( # The $diffHashRef keys to check.
     "copiedFromPath",
     "indexPath",
index 61ca73d6df16fa1a13eb3b8f08465f38b4728445..d2b8c9eea3458e00bf5c5b8ff8e506f29730a0c6 100644 (file)
@@ -36,13 +36,6 @@ use warnings;
 use Test::More;
 use VCSUtils;
 
-my @diffHeaderHashRefKeys = ( # The $diffHeaderHashRef keys to check.
-    "copiedFromPath",
-    "indexPath",
-    "sourceRevision",
-    "svnConvertedText",
-);
-
 # The array of test cases.
 my @testCaseHashRefs = (
 {
@@ -56,7 +49,8 @@ Index: WebKitTools/Scripts/VCSUtils.pm
 @@ -32,6 +32,7 @@ use strict;
  use warnings;
 END
-    # Header keys to check
+    expectedReturn => [
+{
     svnConvertedText => <<'END',
 Index: WebKitTools/Scripts/VCSUtils.pm
 ===================================================================
@@ -65,10 +59,11 @@ Index: WebKitTools/Scripts/VCSUtils.pm
 END
     copiedFromPath => undef,
     indexPath => "WebKitTools/Scripts/VCSUtils.pm",
+    scmFormat => "svn",
     sourceRevision => "53004",
-    # Other values to check
-    lastReadLine => "@@ -32,6 +32,7 @@ use strict;\n",
-    nextLine => " use warnings;\n",
+},
+"@@ -32,6 +32,7 @@ use strict;\n"],
+    expectedNextLine => " use warnings;\n",
 },
 {
     # New test
@@ -81,7 +76,8 @@ Index: WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiffHeader.pl
 @@ -0,0 +1,262 @@
 +#!/usr/bin/perl -w
 END
-    # Header keys to check
+    expectedReturn => [
+{
     svnConvertedText => <<'END',
 Index: WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiffHeader.pl
 ===================================================================
@@ -90,10 +86,11 @@ Index: WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiffHeader.pl
 END
     copiedFromPath => undef,
     indexPath => "WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiffHeader.pl",
+    scmFormat => "svn",
     sourceRevision => undef,
-    # Other values to check
-    lastReadLine => "@@ -0,0 +1,262 @@\n",
-    nextLine => "+#!/usr/bin/perl -w\n",
+},
+"@@ -0,0 +1,262 @@\n"],
+    expectedNextLine => "+#!/usr/bin/perl -w\n",
 },
 {
     # New test
@@ -106,7 +103,8 @@ Index: index_path.py
 @@ -0,0 +1,7 @@
 +# Python file...
 END
-    # Header keys to check
+    expectedReturn => [
+{
     svnConvertedText => <<'END',
 Index: index_path.py
 ===================================================================
@@ -115,10 +113,11 @@ Index: index_path.py
 END
     copiedFromPath => "copied_from_path.py",
     indexPath => "index_path.py",
+    scmFormat => "svn",
     sourceRevision => 53048,
-    # Other values to check
-    lastReadLine => "@@ -0,0 +1,7 @@\n",
-    nextLine => "+# Python file...\n",
+},
+"@@ -0,0 +1,7 @@\n"],
+    expectedNextLine => "+# Python file...\n",
 },
 {
     # New test
@@ -131,7 +130,8 @@ Index: index_path.py\r
 @@ -0,0 +1,7 @@\r
 +# Python file...\r
 END
-    # Header keys to check
+    expectedReturn => [
+{
     svnConvertedText => <<END, # No single quotes to allow interpolation of "\r"
 Index: index_path.py\r
 ===================================================================\r
@@ -140,10 +140,11 @@ Index: index_path.py\r
 END
     copiedFromPath => "copied_from_path.py",
     indexPath => "index_path.py",
+    scmFormat => "svn",
     sourceRevision => 53048,
-    # Other values to check
-    lastReadLine => "@@ -0,0 +1,7 @@\r\n",
-    nextLine => "+# Python file...\r\n",
+},
+"@@ -0,0 +1,7 @@\r\n"],
+    expectedNextLine => "+# Python file...\r\n",
 },
 {
     # New test
@@ -156,7 +157,8 @@ Index: index_path.py
 @@ -0,0 +1,7 @@
 +# Python file...
 END
-    # Header keys to check
+    expectedReturn => [
+{
     svnConvertedText => <<'END',
 Index: index_path.py
 ===================================================================
@@ -165,11 +167,15 @@ Index: index_path.py
 END
     copiedFromPath => "copied_from_path.py",
     indexPath => "index_path.py",
+    scmFormat => "svn",
     sourceRevision => 53048,
-    # Other values to check
-    lastReadLine => "@@ -0,0 +1,7 @@\n",
-    nextLine => "+# Python file...\n",
 },
+"@@ -0,0 +1,7 @@\n"],
+    expectedNextLine => "+# Python file...\n",
+},
+####
+#    Git test cases
+##
 {
     # New test
     diffName => "Git: simple",
@@ -180,7 +186,8 @@ index f5d5e74..3b6aa92 100644
 +++ b/WebCore/rendering/style/StyleFlexibleBoxData.h
 @@ -47,7 +47,6 @@ public:
 END
-    # Header keys to check
+    expectedReturn => [
+{
     svnConvertedText => <<'END',
 Index: WebCore/rendering/style/StyleFlexibleBoxData.h
 index f5d5e74..3b6aa92 100644
@@ -189,10 +196,11 @@ index f5d5e74..3b6aa92 100644
 END
     copiedFromPath => undef,
     indexPath => "WebCore/rendering/style/StyleFlexibleBoxData.h",
+    scmFormat => "git",
     sourceRevision => undef,
-    # Other values to check
-    lastReadLine => "@@ -47,7 +47,6 @@ public:\n",
-    nextLine => undef,
+},
+"@@ -47,7 +47,6 @@ public:\n"],
+    expectedNextLine => undef,
 },
 {
     # New test
@@ -206,7 +214,8 @@ index 0000000..3c9f114
 @@ -0,0 +1,34 @@
 +<html>
 END
-    # Header keys to check
+    expectedReturn => [
+{
     svnConvertedText => <<'END',
 Index: LayoutTests/http/tests/security/listener/xss-inactive-closure.html
 new file mode 100644
@@ -216,11 +225,15 @@ index 0000000..3c9f114
 END
     copiedFromPath => undef,
     indexPath => "LayoutTests/http/tests/security/listener/xss-inactive-closure.html",
+    scmFormat => "git",
     sourceRevision => undef,
-    # Other values to check
-    lastReadLine => "@@ -0,0 +1,34 @@\n",
-    nextLine => "+<html>\n",
 },
+"@@ -0,0 +1,34 @@\n"],
+    expectedNextLine => "+<html>\n",
+},
+####
+#    Binary test cases
+##
 {
     # New test
     diffName => "SVN: binary file",
@@ -238,7 +251,8 @@ Name: svn:mime-type
 
 Q1dTBx0AAAB42itg4GlgYJjGwMDDyODMxMDw34GBgQEAJPQDJA==
 END
-    # Header keys to check
+    expectedReturn => [
+{
     svnConvertedText => <<'END',
 Index: test_file.swf
 ===================================================================
@@ -246,10 +260,11 @@ Cannot display: file marked as a binary type.
 END
     copiedFromPath => undef,
     indexPath => "test_file.swf",
+    scmFormat => "svn",
     sourceRevision => undef,
-    # Other values to check
-    lastReadLine => "svn:mime-type = application/octet-stream\n",
-    nextLine => "\n",
+},
+"svn:mime-type = application/octet-stream\n"],
+    expectedNextLine => "\n",
 },
 {
     # New test
@@ -259,23 +274,15 @@ diff --git a/foo.gif b/foo.gif
 new file mode 100644
 index 0000000000000000000000000000000000000000..64a9532e7794fcd791f6f12157406d9060151690
 GIT binary patch
-literal 512
-zcmZ?wbhEHbRAx|MU|?iW{Kxc~?KofD;ckY;H+&5HnHl!!GQMD7h+sU{_)e9f^V3c?
-zhJP##HdZC#4K}7F68@!1jfWQg2daCm-gs#3|JREDT>c+pG4L<_2;w##WMO#ysPPap
-zLqpAf1OE938xAsSp4!5f-o><?VKe(#0jEcwfHGF4%M1^kRs14oVBp2ZEL{E1N<-zJ
-zsfLmOtKta;2_;2c#^S1-8cf<nb!QnGl>c!Xe6RXvrEtAWBvSDTgTO1j3vA31Puw!A
-zs(87q)j_mVDTqBo-P+03-P5mHCEnJ+x}YdCuS7#bCCyePUe(ynK+|4b-3qK)T?Z&)
-zYG+`tl4h?GZv_$t82}X4*DTE|$;{DEiPyF@)U-1+FaX++T9H{&%cag`W1|zVP@`%b
-zqiSkp6{BTpWTkCr!=<C6Q=?#~R8^JfrliAF6Q^gV9Iup8RqCXqqhqC`qsyhk<-nlB
-z00f{QZvfK&|Nm#oZ0TQl`Yr$BIa6A@16O26ud7H<QM=xl`toLKnz-3h@9c9q&wm|X
-z{89I|WPyD!*M?gv?q`;L=2YFeXrJQNti4?}s!zFo=5CzeBxC69xA<zrjP<wUcCRh4
-ptUl-ZG<%a~#LwkIWv&q!KSCH7tQ8cJDiw+|GV?MN)RjY50RTb-xvT&H
+literal 7
+OcmYex&reDa;sO8*F9L)B
 
 literal 0
 HcmV?d00001
 
 END
-    # Header keys to check
+    expectedReturn => [
+{
     svnConvertedText => <<'END',
 Index: foo.gif
 new file mode 100644
@@ -284,10 +291,11 @@ GIT binary patch
 END
     copiedFromPath => undef,
     indexPath => "foo.gif",
+    scmFormat => "git",
     sourceRevision => undef,
-    # Other values to check
-    lastReadLine => "literal 512\n",
-    nextLine => "zcmZ?wbhEHbRAx|MU|?iW{Kxc~?KofD;ckY;H+&5HnHl!!GQMD7h+sU{_)e9f^V3c?\n",
+},
+"literal 7\n"],
+    expectedNextLine => "OcmYex&reDa;sO8*F9L)B\n",
 },
 {
     # New test
@@ -304,7 +312,8 @@ literal 7
 OcmYex&reD$;sO8*F9L)B
 
 END
-    # Header keys to check
+    expectedReturn => [
+{
     svnConvertedText => <<'END',
 Index: foo.gif
 deleted file mode 100644
@@ -313,75 +322,29 @@ GIT binary patch
 END
     copiedFromPath => undef,
     indexPath => "foo.gif",
+    scmFormat => "git",
     sourceRevision => undef,
-    # Other values to check
-    lastReadLine => "literal 0\n",
-    nextLine => "HcmV?d00001\n",
+},
+"literal 0\n"],
+    expectedNextLine => "HcmV?d00001\n",
 },
 );
 
-# Return the arguments for each assertion per test case.
-#
-# In particular, the number of assertions per test case is the length
-# of the return value of this subroutine on a sample input.
-#
-# Returns @assertionArgsArrayRefs:
-#   $assertionArgsArrayRef: A reference to an array of parameters to pass
-#                           to each call to is(). The parameters are--
-#                             $got: The value obtained
-#                             $expected: The expected value
-#                             $testName: The name of the test
-sub testParseDiffHeaderAssertionArgs($)
-{
-    my ($testCaseHashRef) = @_;
+my $testCasesCount = @testCaseHashRefs;
+plan(tests => 2 * $testCasesCount); # Total number of assertions.
 
-    my $fileHandle;
-    open($fileHandle, "<", \$testCaseHashRef->{inputText});
+foreach my $testCase (@testCaseHashRefs) {
+    my $testNameStart = "parseDiffHeader(): $testCase->{diffName}: comparing";
 
+    my $fileHandle;
+    open($fileHandle, "<", \$testCase->{inputText});
     my $line = <$fileHandle>;
 
-    my ($headerHashRef, $lastReadLine) = VCSUtils::parseDiffHeader($fileHandle, $line);
-
-    my $testNameStart = "parseDiffHeader(): [$testCaseHashRef->{diffName}] ";
-
-    my @assertionArgsArrayRefs; # Return value
-    my $assertionArgsRef;
-
-    foreach my $diffHeaderHashRefKey (@diffHeaderHashRefKeys) {
-        my $testName = "${testNameStart}key=\"$diffHeaderHashRefKey\"";
-        $assertionArgsRef = [$headerHashRef->{$diffHeaderHashRefKey}, $testCaseHashRef->{$diffHeaderHashRefKey}, $testName];
-        push(@assertionArgsArrayRefs, $assertionArgsRef);
-    }
-
-    $assertionArgsRef = [$lastReadLine, $testCaseHashRef->{lastReadLine}, "${testNameStart}lastReadLine"];
-    push(@assertionArgsArrayRefs, $assertionArgsRef);
-
-    my $nextLine = <$fileHandle>;
-    $assertionArgsRef = [$nextLine, $testCaseHashRef->{nextLine}, "${testNameStart}nextLine"];
-    push(@assertionArgsArrayRefs, $assertionArgsRef);
-
-    return @assertionArgsArrayRefs;
-}
-
-# Test parseDiffHeader() for the given test case.
-sub testParseDiffHeader($)
-{
-    my ($testCaseHashRef) = @_;
-
-    my @assertionArgsArrayRefs = testParseDiffHeaderAssertionArgs($testCaseHashRef);
-
-    foreach my $arrayRef (@assertionArgsArrayRefs) {
-        # The parameters are -- is($got, $expected, $testName).
-        is($arrayRef->[0], $arrayRef->[1], $arrayRef->[2]);
-    }
-}
-
-# Count the number of assertions per test case to calculate the total number
-# of Test::More tests. We could have used any test case for the count.
-my $assertionCount = testParseDiffHeaderAssertionArgs($testCaseHashRefs[0]);
+    my @got = VCSUtils::parseDiffHeader($fileHandle, $line);
+    my $expectedReturn = $testCase->{expectedReturn};
 
-plan(tests => @testCaseHashRefs * $assertionCount); # Total number of tests
+    is_deeply(\@got, $expectedReturn, "$testNameStart return value.");
 
-foreach my $testCaseHashRef (@testCaseHashRefs) {
-    testParseDiffHeader($testCaseHashRef);
+    my $gotNextLine = <$fileHandle>;
+    is($gotNextLine, $testCase->{expectedNextLine},  "$testNameStart next read line.");
 }