[Webkit-unassigned] [Bug 44709] deduplicate-tests should be runnable from any WebKit directory

[bugzilla-daemon at webkit.org]

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





--- Comment #17 from Eric Seidel <eric at webkit.org>  2010-09-10 15:53:28 PST ---
(From update of attachment 67251)
View in context: https://bugs.webkit.org/attachment.cgi?id=67251&action=prettypatch

Looks fine.  The attempts to wrap this to 80c make it harder to read than necessary.

> WebKitTools/Scripts/webkitpy/layout_tests/deduplicate_tests.py:61
> +            _log.error("'%s' lacks baseline_search_path(), please fix."
80c does not help readability in all cases.  But I think I lost that battle long ago.  Cant remember. :)

> WebKitTools/Scripts/webkitpy/layout_tests/deduplicate_tests.py:173
> +    """Constructs a relative path to |filename| from |relative_to|.  Also, if
> +    |relative_to| is a sub directory of the layout test directory and
> +    |filename| is not in |relative_to|, return None.  This lets us filter
> +    the results to only show results that are under where the script was run
> +    from.
> +    Args:
> +        filename: The test file we're trying to get a relative path to.
> +        relative_to: The absolute path we're relative to.
> +    Returns:
> +        A relative path to filename or None.
> +    """
This very long docstring ends up basically saying the same thing 3 times.  Especially with the comment about ospath.relpath below it too. :(

> WebKitTools/Scripts/webkitpy/layout_tests/deduplicate_tests.py:180
> +    path = ospath.relpath(os.path.normpath(abs_path),
> +                          os.path.normpath(relative_to))
> +    return path
Just return the result directly, no?

> WebKitTools/Scripts/webkitpy/layout_tests/deduplicate_tests.py:218
> +                yield {'test': test, 'platform': platform,
> +                       'fallback': fallback, 'path': path}
I would have written this as one per line:

yield {
  'test': foo,
   'bar: bar,
}

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