[Webkit-unassigned] [Bug 49835] Ignore reference files which will be used by reftests when collecting test cases.

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Hayato Ito <hayato at chromium.org>  2010-11-22 13:57:19 PST ---
Thank you for the reviews, hamaji-san and Dirk.


(In reply to comment #5)
> (From update of attachment 74426 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=74426&action=review
> 
> > WebKitTools/Scripts/webkitpy/layout_tests/port/test_files.py:123
> > +           in _ignored_suffixes):
> 
> I actually think the any() is fine, but the variable names make it a bit hard to read. I'd use:
> 
> if any(filename.endswith(suffix) for suffix in _ignored_suffixes):
>     _log.warn(...)
>    return True
> return False
> 
> I would also consider renaming is_ignored_file to is_reftest and moving the _ignored_suffixes constant inside the function.
> 
> You could get away with:
> 
> def _is_reftest(filename):
>     return (filename.endswith('-expected.html') or 
>                  filename.endswith('-expected-mismatch.html'))
> 
> Slightly less generic, but a lot easier to read and understand.

I agree. I just renamed that function to 'is_reference_html_file'.  I think that might be better.
It seems I did a too early generalization in the previous patch.

> 
> >> WebKitTools/Scripts/webkitpy/layout_tests/port/test_files.py:124
> >> +        _log.warn('Reftests is not suuperted yet. %s is ignored.', filename)
> > 
> > "Reftest is not" ?
> 
> _log.warn("Ignoring %s - reftests are not supported." % filename)
> 
> or 
> 
> _log.warn("Reftests are not supported - ignoring %s" % filename)
> 
> depending on which aspect you want to emphasize.

Thank you. Done.

> 
> > WebKitTools/Scripts/webkitpy/layout_tests/port/test_files.py:131
> > +    on."""
> 
> I'm not sure that a docstring really adds anything here, but if you really wanted one, you could have "Return True if the filename points to a test file."

Done.

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