[webkit-reviews] review granted: [Bug 75065] run-api-tests should be able to run individual suites and tests : [Attachment 121341] Patch 4 of 5 v2

[bugzilla-daemon at webkit.org]

Adam Roben (:aroben) <aroben at apple.com> has granted David Kilzer (:ddkilzer)
<ddkilzer at webkit.org>'s request for review:
Bug 75065: run-api-tests should be able to run individual suites and tests
https://bugs.webkit.org/show_bug.cgi?id=75065

Attachment 121341: Patch 4 of 5 v2
https://bugs.webkit.org/attachment.cgi?id=121341&action=review

------- Additional Comments from Adam Roben (:aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=121341&action=review


> Tools/Scripts/run-api-tests:117
> +	       print $suite . ":\n";
> +	   }
> +	   print "   " . $test . "\n";

In both of these cases, I'd either use string interpolation:

print "    $test\n";

or multiple arguments to print:

print "    ", $test, "\n";

Explicit concatenation seems a little funny.

> Tools/Scripts/run-api-tests:136
> +	   if ($failed) {
> +	       $anyFailures = 1;
>	   }

I know this isn't new code, but it seems like we could just say $anyFailures
||= $failed.

> Tools/Scripts/run-api-tests:333
> +sub sortTestList($$)
> +{
> +    my ($a, $b) = @_;
> +    my ($suiteA, $testA) = split(/\./, $a);
> +    my ($suiteB, $testB) = split(/\./, $b);
> +    return $suiteA cmp $suiteB ? $suiteA cmp $suiteB : $testA cmp $testB;
> +}

How is this different from the standard sort?

|| in Perl returns the value of the expression that was true-ish, so you can
just say:

return $suiteA cmp $suiteB || $testA cmp $testB;

(Assuming cmp has higher precedence than ||.)