[webkit-reviews] review denied: [Bug 37630] delete redundant test outputs : [Attachment 63990] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Marcus Bulach <bulach at chromium.org>'s
request for review:
Bug 37630: delete redundant test outputs
https://bugs.webkit.org/show_bug.cgi?id=37630

Attachment 63990: Patch
https://bugs.webkit.org/attachment.cgi?id=63990&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
> diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
> +	   * Scripts/deduplicate_tests.py: Added.
> +	   * Scripts/deduplicate_tests_unittest.py: Added.

There are no other unittest.py files or files with _ in the filename in Scripts
and there's only one file that ends in .py.  Maybe it would be better to put
these files in Scripts/webkitpy somewhere and have a wrapper file (e.g.,
duplicate-tests) in Scripts that just imports the files from webkitpy.	I think
the unittest needs to be in webkitpy anyway for Scripts/test-webkitpy to find
it.


> diff --git a/WebKitTools/Scripts/deduplicate_tests.py
b/WebKitTools/Scripts/deduplicate_tests.py
> +# 2.  Redistributions in binary form must reproduce the above copyright
> +#	 notice, this list of conditions and the following disclaimer in the
> +#	 documentation and/or other materials provided with the distribution.

The third clause of the license seems to be missing.  There seems to be some
inconsistency among the existing files in Scripts, but that's a separate issue.
 I would use something like what's in webkit-tools-completion.sh.

> +    # Fill in the map.
> +    cmd = ['git', 'ls-tree', '-r', 'HEAD', 'LayoutTests']

Nit: () instead of [] since cmd doesn't change.

The unittests look great.