RESOLVED FIXED 141152
[webkitpy] Add platform specific Skipped file mechanism for performance tests
https://bugs.webkit.org/show_bug.cgi?id=141152
Summary [webkitpy] Add platform specific Skipped file mechanism for performance tests
Csaba Osztrogonác
Reported 2015-02-02 03:59:38 PST
Now there is only one skipped file for performance tests: PerformanceTests/Skipped. But EFL needs a platform specific one to be able to skip Canvas/reuse.html - bug139812
Attachments
Patch (3.91 KB, patch)
2015-02-02 04:04 PST, Csaba Osztrogonác
no flags
Patch (4.50 KB, patch)
2015-02-02 04:10 PST, Csaba Osztrogonác
no flags
Adds the support for platform modifier (3.54 KB, patch)
2015-02-03 11:53 PST, Ryosuke Niwa
ossy: review+
ossy: commit-queue-
Csaba Osztrogonác
Comment 1 2015-02-02 04:04:06 PST
Created attachment 245867 [details] Patch With this patch we can skip tests for platform ios,mac,win,gtk and efl. I don't want to add nested platforms as LayoutTests has.
Csaba Osztrogonác
Comment 2 2015-02-02 04:10:13 PST
Created attachment 245868 [details] Patch Moved _expectations_from_skipped_files function into skipped_perf_tests, because it isn't used anywhere else.
Ryosuke Niwa
Comment 3 2015-02-02 22:48:53 PST
Comment on attachment 245868 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=245868&action=review > Tools/Scripts/webkitpy/port/base.py:692 > + skipped_file_paths = [self.perf_tests_dir(), self._filesystem.join(self.perf_tests_dir(), 'platform', self.port_name)] > for search_path in skipped_file_paths: Instead of adding a seprate file, can we add a modifier like [EFL] test-name as done in test expectations?
Csaba Osztrogonác
Comment 4 2015-02-02 23:17:24 PST
(In reply to comment #3) > Comment on attachment 245868 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=245868&action=review > > > Tools/Scripts/webkitpy/port/base.py:692 > > + skipped_file_paths = [self.perf_tests_dir(), self._filesystem.join(self.perf_tests_dir(), 'platform', self.port_name)] > > for search_path in skipped_file_paths: > > Instead of adding a seprate file, can we add a modifier like [EFL] test-name > as done in test expectations? Unfortunately no, because PerformanceTests/Skipped file has the simple old style Skipped file syntax, which doesn't support any modifier. _tests_from_skipped_file_contents omits only commented lines, other lines are interpreted as test names: http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/port/base.py?rev=179134#L678 I don't want to implement a sophisticated mechanism for performance tests similar to TestExpectations. I would like to have a workaround until bug139812 is fixed not to have to swith off the EFL performance bot.
Ryosuke Niwa
Comment 5 2015-02-03 11:15:22 PST
(In reply to comment #4) > > Unfortunately no, because PerformanceTests/Skipped file has the simple > old style Skipped file syntax, which doesn't support any modifier. > _tests_from_skipped_file_contents omits only commented lines, other > lines are interpreted as test names: > http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/port/base. > py?rev=179134#L678 There’s no need to limit ourselves to that format. > I don't want to implement a sophisticated mechanism for performance tests > similar to TestExpectations. I would like to have a workaround until > bug139812 is fixed not to have to swith off the EFL performance bot. I don’t like this approach because it introdues “platform” directory, which is otherwise useless.
Ryosuke Niwa
Comment 6 2015-02-03 11:53:01 PST
Created attachment 245951 [details] Adds the support for platform modifier
Csaba Osztrogonác
Comment 7 2015-02-04 02:34:57 PST
Comment on attachment 245951 [details] Adds the support for platform modifier View in context: https://bugs.webkit.org/attachment.cgi?id=245951&action=review LGTM, r=me with some nits. > Tools/Scripts/webkitpy/port/base.py:710 > + line_number = 1 > + tests_to_skip = [] > + for line in skipped_file_contents.split('\n'): We don't need explicit line numbering, enumerate solves this task: for line_number, line in enumerate(skipped_file_contents.split('\n')): > Tools/Scripts/webkitpy/port/base.py:713 > + _log.error("Synatx error at line %d in %s: %s" % (line_number, filename, line)) typo: Synatx -> Syntax
Csaba Osztrogonác
Comment 8 2015-02-04 03:34:57 PST
(In reply to comment #7) > We don't need explicit line numbering, enumerate solves this task: > for line_number, line in enumerate(skipped_file_contents.split('\n')): + start=1 argument for enumerate
Ryosuke Niwa
Comment 9 2015-02-04 10:07:08 PST
Note You need to log in before you can comment on or make changes to this bug.