[Webkit-unassigned] [Bug 27893] run-webkit-tests Skipped Only mode includes tests that are skipped by other flags.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27893


Adam Treat <treat at kde.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34059|review?                     |review-
               Flag|                            |




--- Comment #6 from Adam Treat <treat at kde.org>  2009-08-04 07:58:39 PDT ---
(From update of attachment 34059)
> +2009-07-31  Mike Fenton  <mike.fenton at torchmobile.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Change --skipped=only mode to use $ignoreDirectories to only
> +        run tests that configured by the script, or by the default
> +        values used in the script.

I think the ChangeLog could do a better job of explaining what is happening
here.  Basically, you want --skipped=only to honor the --no-http flag.  That is
not currently happening.  If I understand things correctly, your patch
explicitly fixes this by making --skipped=only honor the ignoreDirectories, one
of which is 'http' if --no-http is passed.

Spell this out as the explicit goal in the ChangeLog and how it was
accomplished.

Also, it is not clear to me that this patch accomplishes what the bug report
says it does: to make --skipped=only honor *all* other flags.  It seems that
this bugfix was rather narrowly tailored to --no-http.  Which is fine!  But it
is good to be clear about this.

> +sub ignoreFileByDirectory {
> +    my($filePath) = @_;
> +    foreach my $ignoredDir (keys %ignoredDirectories) {
> +        if ($filePath =~ m/^$ignoredDir/) {
> +            return 1;
> +        }
> +    }

Perhaps 'fileShouldBeIgnored' instead to be more descriptive?

r- for ChangeLog mostly.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.