Changeset 204549 in webkit


Ignore:
Timestamp:
Aug 16, 2016 7:30:27 PM (8 years ago)
Author:
dbates@webkit.org
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.

Location:
trunk/Tools
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r204544 r204549  
     12016-08-16  Daniel Bates  <dabates@apple.com>
     2
     3        prepare-ChangeLog: Extract logic from generateFunctionLists() into a function that takes a delegate object
     4        https://bugs.webkit.org/show_bug.cgi?id=160924
     5
     6        Reviewed by Stephanie Lewis.
     7
     8        Towards adding unit tests for generateFunctionLists() we move its logic into actuallyGenerateFunctionLists()
     9        and have actuallyGenerateFunctionLists() take a delegate object to use to query the file system and SCM.
     10        We modify generateFunctionLists() to call actuallyGenerateFunctionLists(). This will make is possible to
     11        test the generate function list machinery without requiring a SCM checkout by substituting a delegate
     12        object that mocks out the file system and SCM operations.
     13
     14        * Scripts/VCSUtils.pm:
     15        (parseDiffStartLine): Parses an SVN or Git start line and returns the path to the target file.
     16        * Scripts/prepare-ChangeLog:
     17        (generateFunctionLists): Move functionality to actually generate the function lists to actuallyGenerateFunctionLists(),
     18        abstracting the logic to query the file system and SCM into functions on a delegate object that
     19        we pass to it.
     20        (actuallyGenerateFunctionLists): Extracted from generateFunctionLists().
     21        (diffHeaderFormat): Deleted.
     22
    1232016-08-16  Alex Christensen  <achristensen@webkit.org>
    224

  • trunk/Tools/Scripts/VCSUtils.pm

    r196097 r204549  
    7676        &normalizePath
    7777        &parseChunkRange
     78        &parseDiffStartLine
    7879        &parseFirstEOL
    7980        &parsePatch
    … …  
    668669
    669670    return $fileMode % 2;
     671}
     672
     673# Parses an SVN or Git diff header start line.
     674#
     675# Args:
     676#   $line: "Index: " line or "diff --git" line
     677#
     678# Returns the path of the target file or undef if the $line is unrecognized.
     679sub parseDiffStartLine($)
     680{
     681    my ($line) = @_;
     682    return $1 if $line =~ /$svnDiffStartRegEx/;
     683    return parseGitDiffStartLine($line) if $line =~ /$gitDiffStartRegEx/;
    670684}
    671685

  • trunk/Tools/Scripts/prepare-ChangeLog

    r203453 r204549  
    6565use VCSUtils;
    6666
     67sub actuallyGenerateFunctionLists($$$$$$);
    6768sub attributeCommand($$);
    6869sub changeLogDate($);
    … …  
    7475sub diffCommand($$$$);
    7576sub diffFromToString($$$);
    76 sub diffHeaderFormat();
    7777sub extractLineRangeAfterChange($);
    7878sub extractLineRangeBeforeChange($);
    … …  
    282282{
    283283    my ($changedFiles, $functionLists, $gitCommit, $gitIndex, $mergeBase) = @_;
     284    my %delegateHash = (
     285        openDiff => sub ($$$$) {
     286            my ($changedFiles, $gitCommit, $gitIndex, $mergeBase) = @_;
     287            return unless open(DIFF, "-|", diffCommand($changedFiles, $gitCommit, $gitIndex, $mergeBase));
     288            return \*DIFF;
     289        },
     290        openFile => sub ($) {
     291            my ($file) = @_;
     292            return unless open(SOURCE, "<", $file);
     293            return \*SOURCE;
     294        },
     295        openOriginalFile => sub ($) {
     296            my ($file, $gitCommit, $gitIndex, $mergeBase) = @_;
     297            return unless open(SOURCE, "-|", originalFile($file, $gitCommit, $gitIndex, $mergeBase));
     298            return \*SOURCE;
     299        },
     300        normalizePath => sub ($) {
     301            my ($path) = @_;
     302            return normalizePath(makeFilePathRelative($path));
     303        },
     304    );
     305    actuallyGenerateFunctionLists($changedFiles, $functionLists, $gitCommit, $gitIndex, $mergeBase, \%delegateHash);
     306}
     307
     308sub actuallyGenerateFunctionLists($$$$$$)
     309{
     310    my ($changedFiles, $functionLists, $gitCommit, $gitIndex, $mergeBase, $delegateHashRef) = @_;
    284311
    285312    my %line_ranges_after_changed;
    … …  
    290317        print STDERR "  Reviewing diff to determine which lines changed.\n";
    291318        my $file;
    292         open DIFF, "-|", diffCommand($changedFiles, $gitCommit, $gitIndex, $mergeBase) or die "The diff failed: $!.\n";
    293         while (<DIFF>) {
    294             $file = normalizePath(makeFilePathRelative($1)) if $_ =~ diffHeaderFormat();
     319        my $diffFileHandle = $delegateHashRef->{openDiff}($changedFiles, $gitCommit, $gitIndex, $mergeBase);
     320        if (!$diffFileHandle) {
     321            die "The diff failed: $!.\n";
     322        }
     323        while (<$diffFileHandle>) {
     324            my $filePath = parseDiffStartLine($_);
     325            $file = $delegateHashRef->{normalizePath}($filePath) if $filePath;
    295326            if (defined $file) {
    296327                my ($before_start, $before_end) = extractLineRangeBeforeChange($_);
    … …  
    308339            }
    309340        }
    310         close DIFF;
     341        close($diffFileHandle);
    311342    }
    312343
    … …  
    322353        # Find all the functions in the file.
    323354        if ($line_ranges_after_changed{$file}) {
    324             open(SOURCE, "<", $file) or next;
    325             my @function_ranges = get_function_line_ranges(\*SOURCE, $file);
    326             close SOURCE;
     355            my $sourceFileHandle = $delegateHashRef->{openFile}($file);
     356            next unless $sourceFileHandle;
     357            my @function_ranges = get_function_line_ranges($sourceFileHandle, $file);
     358            close($sourceFileHandle);
    327359
    328360            my @change_ranges = (@{$line_ranges_after_changed{$file}}, []);
    … …  
    336368        }
    337369        # Find the deleted functions in the original file.
    338         if($line_ranges_before_changed{$file}) {
    339             open SOURCE, "-|", originalFile($file, $gitCommit, $gitIndex, $mergeBase) or next;
    340                 my @deleted_function_ranges = get_function_line_ranges(\*SOURCE, $file);
    341             close SOURCE;
     370        if ($line_ranges_before_changed{$file}) {
     371            my $originalFileHandle = $delegateHashRef->{openOriginalFile}($file, $gitCommit, $gitIndex, $mergeBase);
     372            next unless $originalFileHandle;
     373            my @deleted_function_ranges = get_function_line_ranges($originalFileHandle, $file);
     374            close($originalFileHandle);
    342375
    343376            my @change_ranges = (@{$line_ranges_before_changed{$file}}, []);
    … …  
    20492082
    20502083    return $command;
    2051 }
    2052 
    2053 sub diffHeaderFormat()
    2054 {
    2055     return qr/^Index: (\S+)[\r\n]*$/ if isSVN();
    2056     return qr/^diff --git a\/.+ b\/(.+)$/ if isGit();
    20572084}
    20582085
Note: See TracChangeset for help on using the changeset viewer.