Currently, you have to run it from the root of the checkout. Otherwise, the following is raised: File "/src/WebKit/WebKitTools/Scripts/deduplicate-tests", line 84, in <module> main() File "/src/WebKit/WebKitTools/Scripts/deduplicate-tests", line 80, in main run(options) File "/src/WebKit/WebKitTools/Scripts/deduplicate-tests", line 74, in run for dupe in deduplicate_tests.deduplicate(options.glob_pattern): File "/src/chrome/src/third_party/WebKit.git/WebKitTools/Scripts/webkitpy/layout_tests/deduplicate_tests.py", line 195, in deduplicate fallbacks = port_fallbacks() File "/src/chrome/src/third_party/WebKit.git/WebKitTools/Scripts/webkitpy/layout_tests/deduplicate_tests.py", line 55, in port_fallbacks for port_name in os.listdir(os.path.join('LayoutTests', 'platform')): OSError: [Errno 2] No such file or directory: 'LayoutTests/platform'
Created attachment 67116 [details] Patch
(In reply to comment #1) > Created an attachment (id=67116) [details] > Patch This allows deduplicate-tests to be run from any directory in the checkout and will output relative paths so the results can be piped to rm. Also, if you run it in a subdir of LayoutTests, it only outputs files under where you run it from. E.g., if I run the tool while I'm in LayoutTests/win/, I'll only tell me about dupes in LayoutTests/win/.
Comment on attachment 67116 [details] Patch > diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog > index 1807a838c1a39f7b97f4e184509935cc548d7e37..0d9653bc8c75eb021d002ffc84d9d35386e06d78 100644 > --- a/WebKitTools/ChangeLog > +++ b/WebKitTools/ChangeLog > @@ -1,3 +1,13 @@ > +2010-09-09 Tony Chang <tony@chromium.org> > + > + Reviewed by NOBODY (OOPS!). > + > + deduplicate-tests should be runnable from any WebKit directory > + https://bugs.webkit.org/show_bug.cgi?id=44709 > + > + * Scripts/webkitpy/layout_tests/deduplicate_tests.py: > + * Scripts/webkitpy/layout_tests/deduplicate_tests_unittest.py: > + > 2010-09-09 Mihai Parparita <mihaip@chromium.org> > > Reviewed by Tony Chang. > diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/deduplicate_tests.py b/WebKitTools/Scripts/webkitpy/layout_tests/deduplicate_tests.py > index c543d9183a406e78714202e83c99565452357409..beeb680650070a12e0f0f07d8ba80cb9d7860740 100644 > --- a/WebKitTools/Scripts/webkitpy/layout_tests/deduplicate_tests.py > +++ b/WebKitTools/Scripts/webkitpy/layout_tests/deduplicate_tests.py > @@ -36,6 +36,7 @@ import os > import subprocess > import sys > import re > +import webkitpy.common.checkout.scm as scm > import webkitpy.common.system.executive as executive > import webkitpy.common.system.logutils as logutils > import webkitpy.layout_tests.port.factory as port_factory > @@ -56,7 +57,8 @@ def port_fallbacks(): > try: > platforms = port_factory.get(port_name).baseline_search_path() > except NotImplementedError: > - _log.error("'%s' lacks baseline_search_path(), please fix." % port_name) > + _log.error("'%s' lacks baseline_search_path(), please fix." > + % port_name) We don't actually require 80 char lines, but I won't complain if you're trying to maintain them anyway (I do). > fallbacks[port_name] = [_BASE_PLATFORM] > continue > fallbacks[port_name] = [os.path.basename(p) for p in platforms][1:] > @@ -156,11 +158,35 @@ def has_intermediate_results(test, fallbacks, matching_platform, > return False > > > -def find_dups(hashes, port_fallbacks): > +def get_relative_test_path(filename, relative_to): > + """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 path we're relative to. Nit: I might note that relative_to needs to be an absolute path. Otherwise, LGTM although I'm not a reviewer. -- Dirk
Created attachment 67120 [details] Patch
Committed r67216: <http://trac.webkit.org/changeset/67216>
Reverted r67216 for reason: Broke Committed r67218: <http://trac.webkit.org/changeset/67218>
(In reply to comment #6) > Reverted r67216 for reason: > > Broke > > Committed r67218: <http://trac.webkit.org/changeset/67218> Err, the reason got truncated. The reason was that relpath is a python2.6+ feature.
http://trac.webkit.org/changeset/67216 might have broken Chromium Mac Release The following changes are on the blame list: http://trac.webkit.org/changeset/67216 http://trac.webkit.org/changeset/67215 http://trac.webkit.org/changeset/67205 http://trac.webkit.org/changeset/67206 http://trac.webkit.org/changeset/67207
Created attachment 67245 [details] Patch
(In reply to comment #9) > Created an attachment (id=67245) [details] > Patch Same as before except I rolled my own relpath. I tested on Win and Linux and it seems to work.
Comment on attachment 67245 [details] Patch This seems wrong. Did you just want this? http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/system/ospath.py Why not use absolute paths? See how scm.py solves the run-from-anywhere problem by passing a cwd to executive.py: http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/checkout/scm.py
Committed r67241: <http://trac.webkit.org/changeset/67241>
(In reply to comment #12) > Committed r67241: <http://trac.webkit.org/changeset/67241> Sorry, manual error of webkit-patch. Reverting . . .
Reverted r67241 for reason: Accidentally committed Committed r67242: <http://trac.webkit.org/changeset/67242>
Created attachment 67250 [details] Patch
Created attachment 67251 [details] Patch
Comment on attachment 67251 [details] Patch 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, }
Created attachment 67253 [details] Patch
(In reply to comment #11) > (From update of attachment 67245 [details]) > Did you just want this? > http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/system/ospath.py Switched to using ospath.relpath, but the description is wrong. It's like python2.6's os.path.relpath, but it returns None if |path| is outside |start_path|. os.path.relpath is smart enough to put ../ in the returned value. This is good enough for this script, which will now print nothing if you try to run it from, e.g., WebCore or WebKitTools. > Why not use absolute paths? Mainly because it's harder to read and these are really long path names. > See how scm.py solves the run-from-anywhere problem by passing a cwd to executive.py: Good idea, I switch to passing the cwd to run_command and using an absolute path the other place it mattered.
Created attachment 67259 [details] Patch
(In reply to comment #17) > (From update of attachment 67251 [details]) > This very long docstring ends up basically saying the same thing 3 times. Especially with the comment about ospath.relpath below it too. :( Shorted and removed the redundant comment. > > 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? Done. > > 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, > } Done.
Comment on attachment 67259 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=67259&action=prettypatch Looks fine, but I think you want to cache find_checkout_root() > WebKitTools/Scripts/webkitpy/layout_tests/deduplicate_tests.py:111 > + cwd=scm.find_checkout_root()) find_checkout_root is kinda expensive (hits the disk). Maybe your object shoudl cache it somewhere? If this was an actual Command object with a tool.scm, you would just grab tool.scm.checkout_root. :( Bug 45838 is kinda blocking nice non-webkit-patch webkitpy development in this way.
Committed r67588: <http://trac.webkit.org/changeset/67588>