[webkit-reviews] review granted: [Bug 16853] http/tests/loading has different results when run with run-webkit-tests -1 : [Attachment 18438] Run tests in loading/ as if they were invoked with -1 unless a patched DRT exists

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Holger Freyther
<freyther at handhelds.org>'s request for review:
Bug 16853: http/tests/loading has different results when run with
run-webkit-tests -1
http://bugs.webkit.org/show_bug.cgi?id=16853

Attachment 18438: Run tests in loading/ as if they were invoked with -1 unless
a patched DRT exists
http://bugs.webkit.org/attachment.cgi?id=18438&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
r=me, even though I am worried about slowing down regression tests.

+	 postProcessOneTest($old_base, 1) unless isQt();

I think the fact that isQt is a list of DumpRenderTree implementations that
support a new mode is quite unclear here. Maybe there's a better way of writing
this?

+    if ($test =~ /loading\//) {

This seems like it will fire for any directory with a suffix of "loading" --
maybe the expression should be more specific?

+	 postProcessOneTest($old_base, 1) unless isQt();

Would be nicer to say "true" rather than "1" here, and even nicer to have a
name rather than just a magic value -- maybe a different function name for
this?

+    my ($base,$forceClose) = @_;

Missing space after comma.

+    if (($count + 1) % $testsPerDumpTool == 0 || $count == $#tests ||
$forceClose == 1) {

No need for "== 1" here. I think it makes things more confusing.