Extract PerfTestFactory
Created attachment 137148 [details] cleanup
Comment on attachment 137148 [details] cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=137148&action=review > Tools/Scripts/webkitpy/performance_tests/perftest.py:186 > + pattern_map = [ _pattern_map? > Tools/Scripts/webkitpy/performance_tests/perftest.py:192 > + def create_perf_test(cls, test_name, path): Do you need to assert() that the path is relative? absolute paths would do the wrong thing here. :) > Tools/Scripts/webkitpy/performance_tests/perftest_unittest.py:62 > + test = PerfTest('some-test', '/path/some-dir/some-test') Seems like a bad test case, since you don't use absolute paths, normally. > Tools/Scripts/webkitpy/performance_tests/perftest_unittest.py:128 > + test = PerfTestFactory.create_perf_test('inspector/some-test', '/path/inspector/some-test') I don't see how these pass? That doesn't match ^inspector ?? > Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:131 > + tests.append(PerfTestFactory.create_perf_test(relative_path.replace('\\', '/'), path)) What's with the windows hack here?? Don't we have a nicer way to ensure unix style paths?
(In reply to comment #2) > (From update of attachment 137148 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=137148&action=review > > > Tools/Scripts/webkitpy/performance_tests/perftest.py:186 > > + pattern_map = [ > > _pattern_map? Will do. > > Tools/Scripts/webkitpy/performance_tests/perftest.py:192 > > + def create_perf_test(cls, test_name, path): > > Do you need to assert() that the path is relative? absolute paths would do the wrong thing here. :) No, they need to be absolute paths. > > Tools/Scripts/webkitpy/performance_tests/perftest_unittest.py:128 > > + test = PerfTestFactory.create_perf_test('inspector/some-test', '/path/inspector/some-test') > > I don't see how these pass? That doesn't match ^inspector ?? The regex matches the test name, not the path. > > Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:131 > > + tests.append(PerfTestFactory.create_perf_test(relative_path.replace('\\', '/'), path)) > > What's with the windows hack here?? Don't we have a nicer way to ensure unix style paths? I'd love to know if there's one. The only reason I have that hack is so that test names match between different platforms.
Created attachment 137160 [details] Renamed pattern_map to _pattern_map
Comment on attachment 137160 [details] Renamed pattern_map to _pattern_map View in context: https://bugs.webkit.org/attachment.cgi?id=137160&action=review > Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:131 > + tests.append(PerfTestFactory.create_perf_test(relative_path.replace('\\', '/'), path)) I think you need to convert the \ to / before you call skips_perf_test on line 129, right?
Thanks for the review. (In reply to comment #5) > (From update of attachment 137160 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=137160&action=review > > > Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:131 > > + tests.append(PerfTestFactory.create_perf_test(relative_path.replace('\\', '/'), path)) > > I think you need to convert the \ to / before you call skips_perf_test on line 129, right? Yes.
Committed r115410: <http://trac.webkit.org/changeset/115410>