Bug 87376 – [Qt] NRWT and ORWT checks only for Skipped in platform/qt-5.0-wk1

• Component: WebKit Qt
• Status: RESOLVED FIXED

[János Badics]

Running NRWT or ORWT with --platform qt[+ suffixes] uses a set of baseline search paths for expectations where qt-5.0-wk1 is not included. When run with qt5.0, it should be included to handle qt-5.0 + WebKit1 specific expectations.

[János Badics]

Created attachment 143798 [details]
proposed patch

[Csaba Osztrogonác]

Comment on attachment 143798 [details]
proposed patch

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

Please add/check unit test for the NRWT change.

> Tools/ChangeLog:7
> +        Reviewed by Csaba Osztrogonác.

It isn't true now. :)

> Tools/Scripts/old-run-webkit-tests:2168
> +        if ($platform eq "qt-5.0-wk1" || $platform eq "qt-5.0" || ($platform eq "qt" && getQtVersion() eq "5.0")) {
> +            push @platforms, "qt-5.0-wk1";
> +        }
> +

It's strange for me a little bit ... I should check this part of the code.

[János Badics]

Created attachment 143822 [details]
proposed patch

- Added unittest to the modifications of qt.py
- Made baseline path determining more simple on orwt
- Corrected changelog

[János Badics]

Created attachment 143823 [details]
proposed patch

see above (the previous patch contained trailing whitespace)

[Csaba Osztrogonác]

Comment on attachment 143823 [details]
proposed patch

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

Could you check if my suggestions are correct?

> Tools/Scripts/old-run-webkit-tests:2166
>          if ($platform eq "qt-5.0-wk2") {
>              push @platforms, $platform;
>          }
> +        elsif (getQtVersion() eq "5.0") {
> +            push @platforms, "qt-5.0-wk1"
> +        }

It isn't the best solution ... We don't want to use qt-5.0-wk1 Skipped list for Qt 4.8.

What do you think about this one?

if ($platform eq "qt-5.0-wk2" || getQtVersion() eq "5.0" && $useWebKitTestRunner) {
    push @platforms, "qt-5.0-wk2";
}
elsif ($platform eq "qt-5.0-wk1" || getQtVersion() eq "5.0" && !$useWebKitTestRunner) {
    push @platforms, "qt-5.0-wk1"
}

> Tools/Scripts/webkitpy/layout_tests/port/qt.py:118
>          if self.get_option('webkit_test_runner'):
>              search_paths.append(self._wk2_port_name())
> +        elif '5.0' in version:
> +            search_paths.append('qt-5.0-wk1')

It isn't so nice ...

What do you think about this one?

if '5.0' in version:
    if self.get_option('webkit_test_runner'):
        search_paths.append('qt-5.0-wk2')
    else:
        search_paths.append('qt-5.0-wk1')

[János Badics]

Created attachment 144034 [details]
proposed patch

Modified previous patch based on suggestions of Csaba Osztrogonác.

[Dirk Pranke]

Comment on attachment 144034 [details]
proposed patch

patch looks fine to me but one of the Qt folks might want to give the final approval.

[Csaba Osztrogonác]

Comment on attachment 144034 [details]
proposed patch

r=me

[Csaba Osztrogonác]

Landed in http://trac.webkit.org/changeset/118898