Bug 89108 – [Qt][NRWT] Enable cascaded TestExpectations

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

[Csaba Osztrogonác]

SSIA

[Csaba Osztrogonác]

Created attachment 147605 [details]
Patch

WIP patch

[Peter Gal]

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.

[Csaba Osztrogonác]

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

[János Badics]

Created attachment 150802 [details]
proposed patch

Unified assert paths in one variable and refactored expectations_files() a bit.

[János Badics]

Created attachment 150897 [details]
proposed patch

Corrected two minor mistakes in the previous patch

[Dirk Pranke]

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())

[János Badics]

Created attachment 151215 [details]
proposed patch

Corrected previous patch according to the recommendation of Dirk Pranke.

[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.

[WebKit Review Bot]

Comment on attachment 151215 [details]
proposed patch

Clearing flags on attachment: 151215

Committed r122124: <http://trac.webkit.org/changeset/122124>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by 90815

[Csaba Osztrogonác]

(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)

[Csaba Osztrogonác]

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

[János Badics]

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.

[Csaba Osztrogonác]

Comment on attachment 151443 [details]
proposed patch

qt-win and qt-mac is still missing ...

[János Badics]

Created attachment 151444 [details]
proposed patch

Added Expectations files to qt-arm, qt-mac and qt-win as well.

[Csaba Osztrogonác]

Comment on attachment 151444 [details]
proposed patch

Landed in r122217.

[Csaba Osztrogonác]

Comment on attachment 151444 [details]
proposed patch

It was landed ... why did you set cq? again?