[webkit-reviews] review granted: [Bug 61088] run-api-tests should run one test per process : [Attachment 93974] Small formatting issue

[bugzilla-daemon at webkit.org]

Adam Roben (:aroben) <aroben at apple.com> has granted Dmitry Lomov
<dslomov at google.com>'s request for review:
Bug 61088: run-api-tests should run one test per process
https://bugs.webkit.org/show_bug.cgi?id=61088

Attachment 93974: Small formatting issue
https://bugs.webkit.org/attachment.cgi?id=93974&action=review

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

> Tools/ChangeLog:8
> +2011-05-18  Dmitry Lomov  <dslomov at google.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   run-api-tests should run one test per process
> +	   https://bugs.webkit.org/show_bug.cgi?id=61088
> +
> +	   * Scripts/run-api-tests:

It would be good to have some more comments in here explaining where this code
came from. I assume a lot of it is from an older version of run-api-tests.

> Tools/Scripts/run-api-tests:86
> +if (runAllTests()) {
> +    exit 1;
> +}
> +else {
> +    exit 0;
> +}

"else" should be on the same line as "}". But there's no need for "else" after
"exit".

> Tools/Scripts/run-api-tests:181
> +    f ($result == 0) {

"f"?

We normally say "!$result" instead of "$result == 0".

> Tools/Scripts/run-api-tests:207
> +	   unless ($verbose) {
> +	       open(DEVNULL, ">", File::Spec->devnull()) or die "Failed to open
/dev/null";
> +	       $childErr = ">&DEVNULL";
> +	   } else {
> +	       $childErr = ">&STDERR";
> +	   }

I find "unless/else" confusing. "if/else" hurts my tiny head much less.

> Tools/Scripts/run-api-tests:297
> -
> +    

You should undo this change.