Bug 89108

Created attachment 147605 [details] Patch WIP patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147605&action=review > Tools/Scripts/webkitpy/layout_tests/port/qt.py:153 > + x.append(self._filesystem.join(self._webkit_baseline_path('qt'), 'TestExpectations')) > + x.append(self._filesystem.join(self._webkit_baseline_path(self.name()), 'TestExpectations')) > + version = self.qt_version() > + if '4.8' in version: > + x.append(self._filesystem.join(self._webkit_baseline_path('qt-4.8'), 'TestExpectations')) > + elif version: > + x.append(self._filesystem.join(self._webkit_baseline_path('qt-5.0'), 'TestExpectations')) > + if self.get_option('webkit_test_runner'): > + x.append(self._filesystem.join(self._webkit_baseline_path('qt-5.0-wk2'), 'TestExpectations')) > + else: > + x.append(self._filesystem.join(self._webkit_baseline_path('qt-5.0-wk1'), 'TestExpectations')) Just a tipp: we should first collect out the names of the platforms and after that build up the paths. That way there will be less path joins written down and the code should be more readable. > Tools/Scripts/webkitpy/layout_tests/port/qt_unittest.py:56 > + def _assert_expectations_files(self, expectations_paths, os_name=None, use_webkit2=False, qt_version='4.8'): We should require the os_name, no need for default arument. > Tools/Scripts/webkitpy/layout_tests/port/qt_unittest.py:62 > + port_name = 'qt-' + os_name If os_name is not specified, then with the default None this'll raise an exception.

we should fix the baseline search path in another bug: https://bugs.webkit.org/show_bug.cgi?id=89882 (before this patch)

Created attachment 150802 [details] proposed patch Unified assert paths in one variable and refactored expectations_files() a bit.

Created attachment 150897 [details] proposed patch Corrected two minor mistakes in the previous patch

Comment on attachment 150897 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=150897&action=review > Tools/Scripts/webkitpy/layout_tests/port/qt.py:149 > + return paths Nit: I'd probably write this in one line using a list comprehension: return reversed(self._filesystem.join(self._webkit_baseline_path(p), 'TestExpectations') for p in self._search_paths())

Created attachment 151215 [details] proposed patch Corrected previous patch according to the recommendation of Dirk Pranke.

Comment on attachment 151215 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=151215&action=review > Tools/Scripts/webkitpy/layout_tests/port/qt.py:146 > + return list(reversed([self._filesystem.join(self._webkit_baseline_path(p), 'TestExpectations') for p in self._search_paths()])) Minor nits ... you don't need either the list() or the nested [ ] around the list comprehensions, but we can remove those in a subsequent cleanup.

Comment on attachment 151215 [details] proposed patch Clearing flags on attachment: 151215 Committed r122124: <http://trac.webkit.org/changeset/122124>

All reviewed patches have been landed. Closing bug.

Re-opened since this is blocked by 90815

(In reply to comment #11) > Re-opened since this is blocked by 90815 It broke the NRWT: 11:56:31.757 24122 Parsing expectations ... 11:56:31.771 24122 File "/ramdisk/qt-linux-64-release-quicktest/build/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 127, in run 11:56:31.772 24122 manager.parse_expectations() 11:56:31.772 24122 File "/ramdisk/qt-linux-64-release-quicktest/build/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 374, in parse_expectations 11:56:31.772 24122 self._expectations = test_expectations.TestExpectations(self._port, self._test_files) 11:56:31.772 24122 File "/ramdisk/qt-linux-64-release-quicktest/build/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py", line 732, in __init__ 11:56:31.772 24122 expectations_dict = port.expectations_dict() 11:56:31.772 24122 File "/ramdisk/qt-linux-64-release-quicktest/build/Tools/Scripts/webkitpy/layout_tests/port/base.py", line 911, in expectations_dict 11:56:31.772 24122 expectations[path] = self._filesystem.read_text_file(path) 11:56:31.772 24122 File "/ramdisk/qt-linux-64-release-quicktest/build/Tools/Scripts/webkitpy/common/system/filesystem.py", line 218, in read_text_file 11:56:31.772 24122 with codecs.open(path, 'r', 'utf8') as f: 11:56:31.772 24122 File "/usr/lib/python2.6/codecs.py", line 881, in open 11:56:31.772 24122 file = __builtin__.open(filename, mode, buffering)

Let's check what happened tomorrow. And please _run_ layout tests before uploading an NRWT patch next time, not only unittests ...

Created attachment 151443 [details] proposed patch Sorry, I forgot to include TestExpectations file. That's why my previous patch caused problems in the layout tests. This patch includes the additional (and empty) expectations files in the qt-5.0-wk1, qt-5.0-wk2, qt-5.0, qt-4.8 and qt-linux directories.

Comment on attachment 151443 [details] proposed patch qt-win and qt-mac is still missing ...

Created attachment 151444 [details] proposed patch Added Expectations files to qt-arm, qt-mac and qt-win as well.

Comment on attachment 151444 [details] proposed patch Landed in r122217.

Comment on attachment 151444 [details] proposed patch It was landed ... why did you set cq? again?