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

[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 120276: Patch 5 of 5
https://bugs.webkit.org/attachment.cgi?id=120276&action=review

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


r+/cq- since this will likely change based on how you revise patch 4.

> Tools/Scripts/run-api-tests:79
> -    'suite=s@' => \$suiteNames
> +    'suite=s@' => \$suiteNames,
> +    'test=s@' => \$testNames

Same comments as before about using an array and a trailing comma.

> Tools/Scripts/run-api-tests:120
> -if ($suiteNames) {
> +if ($suiteNames || $testNames) {
>      my %tests;
> -    for my $suite (@$suiteNames) {
> -	   if (exists $allTests{$suite}) {
> -	       $tests{$suite} = $allTests{$suite};
> -	   } else {
> -	       warn "Suite '$suite' does not exist!";
> +    if ($suiteNames) {
> +	   for my $suite (@$suiteNames) {
> +	       if (exists $allTests{$suite}) {
> +		   $tests{$suite} = $allTests{$suite};
> +	       } else {
> +		   warn "Suite '$suite' does not exist!";
> +	       }
> +	   }
> +    }
> +    if ($testNames) {
> +	   for my $testName (@$testNames) {
> +	       my ($suite, $test) = split(/\./, $testName);
> +	       if (!exists $allTests{$suite}) {
> +		   warn "Suite '$suite' does not exist!";
> +	       } elsif (!grep($test, @{$allTests{$suite}})) {
> +		   warn "Test '$testName' in suite '$suite' does not exist!";
> +	       } else {
> +		   push(@{$tests{$suite}}, $test);
> +	       }
>	   }
>      }

It seems like this would all be a lot simpler if we stored "suite.test" names
in an array instead of the %allTests hash. Then you could just use grep for
everything!