Bug 78012 – run-perf-tests doesn't recognize paths that start with PerformanceTests

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Ryosuke Niwa]

run-perf-tests PerformanceTests/Parser runs exactly 0 tests because run-perf-tests recognizes that as PerformanceTests/PerformanceTests/Parser. It's annoying as hell.

[Adam Barth]

run-webkit-tests has some code to remove the LayoutTests from command line arguments.  Presumably you want something similar here.

[Ryosuke Niwa]

(In reply to comment #1)
> run-webkit-tests has some code to remove the LayoutTests from command line arguments.  Presumably you want something similar here.

Something like that. I just looked at its implementation in manager.py and it seems way too complicated for run-perf-test. I'm just going to use filesystem.relpath instead.

[Ryosuke Niwa]

Created attachment 125942 [details]
Fixes the bug

[Ryosuke Niwa]

I feel like we should be able to use the same approach in run-webkit-tests but there's some special treatment for / and \ there so not sure if that's possible.

[Adam Barth]

Comment on attachment 125942 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=125942&action=review

> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:111
> +            if self._host.filesystem.exists(arg):

It's strange that this checks the real file system for existence.  I would expect this to be a syntactic transformation that doesn't query the local file system.

[Ryosuke Niwa]

Comment on attachment 125942 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=125942&action=review

>> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:111
>> +            if self._host.filesystem.exists(arg):
> 
> It's strange that this checks the real file system for existence.  I would expect this to be a syntactic transformation that doesn't query the local file system.

Otherwise how would I figure out whether we should use relpath or not? An alternative is to add both since find_files.find only finds existing files.

[Adam Barth]

> Otherwise how would I figure out whether we should use relpath or not? An alternative is to add both since find_files.find only finds existing files.

That could work.  You can also look whether the string PerformanceTests appears in the path.

[Ryosuke Niwa]

(In reply to comment #7)
> > Otherwise how would I figure out whether we should use relpath or not? An alternative is to add both since find_files.find only finds existing files.
> 
> That could work.  You can also look whether the string PerformanceTests appears in the path.

I want to avoid hard-coding "PerformanceTests" since I'm hoping that I can also replace the equivalent function in manager.py by this one as a followup.

[Hajime Morrita]

(In reply to comment #8)
> I want to avoid hard-coding "PerformanceTests" since I'm hoping that I can also replace the equivalent function in manager.py by this one as a followup.
You can just pass the "base directory name" as an argument to the (coming shared) function.
Avoiding file system access is good for performance reason. In this case, startup speed.

Anyway, I like this change. I'm tired to remove "PerformanceTest" part
from my completed-by-the-shell path ;-)

[Ryosuke Niwa]

Created attachment 126024 [details]
Updated per Adam's comment

[Adam Barth]

Comment on attachment 126024 [details]
Updated per Adam's comment

That certainly looks nicer.  :)

[Ryosuke Niwa]

Committed r107053: <http://trac.webkit.org/changeset/107053>