Log Message
prepare-ChangeLog: Extract logic from generateFunctionLists() into a function that takes a delegate object https://bugs.webkit.org/show_bug.cgi?id=160924
Reviewed by Stephanie Lewis. Towards adding unit tests for generateFunctionLists() we move its logic into actuallyGenerateFunctionLists() and have actuallyGenerateFunctionLists() take a delegate object to use to query the file system and SCM. We modify generateFunctionLists() to call actuallyGenerateFunctionLists(). This will make is possible to test the generate function list machinery without requiring a SCM checkout by substituting a delegate object that mocks out the file system and SCM operations. * Scripts/VCSUtils.pm: (parseDiffStartLine): Parses an SVN or Git start line and returns the path to the target file. * Scripts/prepare-ChangeLog: (generateFunctionLists): Move functionality to actually generate the function lists to actuallyGenerateFunctionLists(), abstracting the logic to query the file system and SCM into functions on a delegate object that we pass to it. (actuallyGenerateFunctionLists): Extracted from generateFunctionLists(). (diffHeaderFormat): Deleted.
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (204548 => 204549)
--- trunk/Tools/ChangeLog 2016-08-17 02:29:30 UTC (rev 204548)
+++ trunk/Tools/ChangeLog 2016-08-17 02:30:27 UTC (rev 204549)
@@ -1,3 +1,25 @@
+2016-08-16 Daniel Bates <[email protected]>
+
+ prepare-ChangeLog: Extract logic from generateFunctionLists() into a function that takes a delegate object
+ https://bugs.webkit.org/show_bug.cgi?id=160924
+
+ Reviewed by Stephanie Lewis.
+
+ Towards adding unit tests for generateFunctionLists() we move its logic into actuallyGenerateFunctionLists()
+ and have actuallyGenerateFunctionLists() take a delegate object to use to query the file system and SCM.
+ We modify generateFunctionLists() to call actuallyGenerateFunctionLists(). This will make is possible to
+ test the generate function list machinery without requiring a SCM checkout by substituting a delegate
+ object that mocks out the file system and SCM operations.
+
+ * Scripts/VCSUtils.pm:
+ (parseDiffStartLine): Parses an SVN or Git start line and returns the path to the target file.
+ * Scripts/prepare-ChangeLog:
+ (generateFunctionLists): Move functionality to actually generate the function lists to actuallyGenerateFunctionLists(),
+ abstracting the logic to query the file system and SCM into functions on a delegate object that
+ we pass to it.
+ (actuallyGenerateFunctionLists): Extracted from generateFunctionLists().
+ (diffHeaderFormat): Deleted.
+
2016-08-16 Alex Christensen <[email protected]>
URLParser should parse URLs without credentials
Modified: trunk/Tools/Scripts/VCSUtils.pm (204548 => 204549)
--- trunk/Tools/Scripts/VCSUtils.pm 2016-08-17 02:29:30 UTC (rev 204548)
+++ trunk/Tools/Scripts/VCSUtils.pm 2016-08-17 02:30:27 UTC (rev 204549)
@@ -75,6 +75,7 @@
&mergeChangeLogs
&normalizePath
&parseChunkRange
+ &parseDiffStartLine
&parseFirstEOL
&parsePatch
&pathRelativeToSVNRepositoryRootForPath
@@ -669,6 +670,19 @@
return $fileMode % 2;
}
+# Parses an SVN or Git diff header start line.
+#
+# Args:
+# $line: "Index: " line or "diff --git" line
+#
+# Returns the path of the target file or undef if the $line is unrecognized.
+sub parseDiffStartLine($)
+{
+ my ($line) = @_;
+ return $1 if $line =~ /$svnDiffStartRegEx/;
+ return parseGitDiffStartLine($line) if $line =~ /$gitDiffStartRegEx/;
+}
+
# Parse the Git diff header start line.
#
# Args:
Modified: trunk/Tools/Scripts/prepare-ChangeLog (204548 => 204549)
--- trunk/Tools/Scripts/prepare-ChangeLog 2016-08-17 02:29:30 UTC (rev 204548)
+++ trunk/Tools/Scripts/prepare-ChangeLog 2016-08-17 02:30:27 UTC (rev 204549)
@@ -64,6 +64,7 @@
use POSIX qw(strftime);
use VCSUtils;
+sub actuallyGenerateFunctionLists($$$$$$);
sub attributeCommand($$);
sub changeLogDate($);
sub changeLogEmailAddressFromArgs($$);
@@ -73,7 +74,6 @@
sub determinePropertyChanges($$$);
sub diffCommand($$$$);
sub diffFromToString($$$);
-sub diffHeaderFormat();
sub extractLineRangeAfterChange($);
sub extractLineRangeBeforeChange($);
sub fetchBugXMLData($$);
@@ -281,7 +281,34 @@
sub generateFunctionLists($$$$$)
{
my ($changedFiles, $functionLists, $gitCommit, $gitIndex, $mergeBase) = @_;
+ my %delegateHash = (
+ openDiff => sub ($$$$) {
+ my ($changedFiles, $gitCommit, $gitIndex, $mergeBase) = @_;
+ return unless open(DIFF, "-|", diffCommand($changedFiles, $gitCommit, $gitIndex, $mergeBase));
+ return \*DIFF;
+ },
+ openFile => sub ($) {
+ my ($file) = @_;
+ return unless open(SOURCE, "<", $file);
+ return \*SOURCE;
+ },
+ openOriginalFile => sub ($) {
+ my ($file, $gitCommit, $gitIndex, $mergeBase) = @_;
+ return unless open(SOURCE, "-|", originalFile($file, $gitCommit, $gitIndex, $mergeBase));
+ return \*SOURCE;
+ },
+ normalizePath => sub ($) {
+ my ($path) = @_;
+ return normalizePath(makeFilePathRelative($path));
+ },
+ );
+ actuallyGenerateFunctionLists($changedFiles, $functionLists, $gitCommit, $gitIndex, $mergeBase, \%delegateHash);
+}
+sub actuallyGenerateFunctionLists($$$$$$)
+{
+ my ($changedFiles, $functionLists, $gitCommit, $gitIndex, $mergeBase, $delegateHashRef) = @_;
+
my %line_ranges_after_changed;
my %line_ranges_before_changed;
if (@$changedFiles) {
@@ -289,9 +316,13 @@
# Use line numbers from the "after" side of each diff.
print STDERR " Reviewing diff to determine which lines changed.\n";
my $file;
- open DIFF, "-|", diffCommand($changedFiles, $gitCommit, $gitIndex, $mergeBase) or die "The diff failed: $!.\n";
- while (<DIFF>) {
- $file = normalizePath(makeFilePathRelative($1)) if $_ =~ diffHeaderFormat();
+ my $diffFileHandle = $delegateHashRef->{openDiff}($changedFiles, $gitCommit, $gitIndex, $mergeBase);
+ if (!$diffFileHandle) {
+ die "The diff failed: $!.\n";
+ }
+ while (<$diffFileHandle>) {
+ my $filePath = parseDiffStartLine($_);
+ $file = $delegateHashRef->{normalizePath}($filePath) if $filePath;
if (defined $file) {
my ($before_start, $before_end) = extractLineRangeBeforeChange($_);
if ($before_start >= 1 && $before_end >= 1) {
@@ -307,7 +338,7 @@
}
}
}
- close DIFF;
+ close($diffFileHandle);
}
# For each source file, convert line range to function list.
@@ -321,9 +352,10 @@
my %saw_function;
# Find all the functions in the file.
if ($line_ranges_after_changed{$file}) {
- open(SOURCE, "<", $file) or next;
- my @function_ranges = get_function_line_ranges(\*SOURCE, $file);
- close SOURCE;
+ my $sourceFileHandle = $delegateHashRef->{openFile}($file);
+ next unless $sourceFileHandle;
+ my @function_ranges = get_function_line_ranges($sourceFileHandle, $file);
+ close($sourceFileHandle);
my @change_ranges = (@{$line_ranges_after_changed{$file}}, []);
my @functions = generateFunctionListsByRanges($file, \@change_ranges, \@function_ranges, \%saw_function);
@@ -335,10 +367,11 @@
}
}
# Find the deleted functions in the original file.
- if($line_ranges_before_changed{$file}) {
- open SOURCE, "-|", originalFile($file, $gitCommit, $gitIndex, $mergeBase) or next;
- my @deleted_function_ranges = get_function_line_ranges(\*SOURCE, $file);
- close SOURCE;
+ if ($line_ranges_before_changed{$file}) {
+ my $originalFileHandle = $delegateHashRef->{openOriginalFile}($file, $gitCommit, $gitIndex, $mergeBase);
+ next unless $originalFileHandle;
+ my @deleted_function_ranges = get_function_line_ranges($originalFileHandle, $file);
+ close($originalFileHandle);
my @change_ranges = (@{$line_ranges_before_changed{$file}}, []);
my @functions = generateFunctionListsByRanges($file, \@change_ranges, \@deleted_function_ranges, \%saw_function);
@@ -2050,12 +2083,6 @@
return $command;
}
-sub diffHeaderFormat()
-{
- return qr/^Index: (\S+)[\r\n]*$/ if isSVN();
- return qr/^diff --git a\/.+ b\/(.+)$/ if isGit();
-}
-
sub findOriginalFileFromSvn($)
{
my ($file) = @_;
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
