[webkit-reviews] review requested: [Bug 73079] Web Inspector: chromium: I'd like to add a script for running perf tests for WebInspector. : [Attachment 116910] [patch] next iteration.

[bugzilla-daemon at webkit.org]

Ilya Tikhonovsky <loislo at chromium.org> has asked  for review:
Bug 73079: Web Inspector: chromium: I'd like to add a script for running perf
tests for WebInspector.
https://bugs.webkit.org/show_bug.cgi?id=73079

Attachment 116910: [patch] next iteration.
https://bugs.webkit.org/attachment.cgi?id=116910&action=review

------- Additional Comments from Ilya Tikhonovsky <loislo at chromium.org>
(In reply to comment #12)
> (From update of attachment 116801 [details])
> 
> > Tools/Scripts/webkitpy/common/test_files.py:35
> > +under that directory."""
> 
> If this file is going to be re-used by both layout tests and perf tests, you
need to update this docstring. I would be tempted to rename it to something
other than "test_files.py" (maybe find_files.py? That's not very good, either).


renamed to find_files.py

> 
> > Tools/Scripts/webkitpy/common/test_files.py:52
> > +	       everything under the base_dir.
> 
> Add skipped_directories, file_filter to the docstring.

done

> 
> > Tools/Scripts/webkitpy/layout_tests/port/test_files.py:36
> >  
> 
> I would probably merge all of the remaining code in this file back into
port/base.py, and delete this file. That will solve your "two files with the
same name" problem and make the layering clearer, I think.
> 
> > Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:54
> > +	     self._host._initialize_scm()
> 
> Question for Eric ... should this be a public function? Is it supposed to be
something that is optional, or should this be called as part of the
constructor?