Bug 174092 – [WPE] Fix layout test baseline and TestExpectations hierarchy

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Zan Dobersek]

[WPE] Only use the WPE-specific baseline and TestExpectations files

[Zan Dobersek]

Created attachment 314474 [details]
Patch

[Michael Catanzaro]

Comment on attachment 314474 [details]
Patch

I don't understand this. WPE is WK2, so it should use the WK2 baselines, and override them where they don't work for WPE. Just like GTK.

Why should WPE be special here?

The GTK EWS does not run tests anymore either. That needs fixed, but only if we can actually keep the tree green. Same goes for WPE, right?

[Michael Catanzaro]

Comment on attachment 314474 [details]
Patch

I don't think we should do this.

[Michael Catanzaro]

We are currently having some fun trying to set resourceLoadStatistics expectations for WPE. It seems the WK2 expectations are currently overriding the WPE expectations, which is broken. It should be the opposite.

See r219256, which halved failures on the WPE bot only because I forgot to annotate the tests as [ Pass ] in the WK2 expectations, and my fix in r219258, which is correct but caused the failures to return. The Timeout expectations in the WPE expectations file are being ignored in favor of the WK2 expectations.

[Carlos Alberto Lopez Perez]

I'm also wondering why WPE should not inherit the WK2 expectations ??

[Carlos Alberto Lopez Perez]

I think we should aim at having a common base between both ports, to not duplicate efforts more than necessary.

So, in the future, having a glib expectations that both GTK and WPE will inherit makes sense to me.

Something like:

* Baseline search path: platform/gtk -> platform/glib -> platform/wk2 -> generic
* Baseline search path: platform/wpe -> platform/glib -> platform/wk2 -> generic

[Michael Catanzaro]

Yes, that would be ideal.

At any rate, the immediate problem is to fix the nonsense WPE search path.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #4)
> See r219256, which halved failures on the WPE bot only because I forgot to
> annotate the tests as [ Pass ] in the WK2 expectations, and my fix in
> r219258, which is correct but caused the failures to return.

It got rolled out because it was not actually correct. :) It introduced failures for Apple. There must be more failing tests there that were not individually marked in the WK2 expectations file. Whoops!

[Michael Catanzaro]

(In reply to Carlos Alberto Lopez Perez from comment #5)
> I'm also wondering why WPE should not inherit the WK2 expectations ??

The problem is the default expectations path is this:

wpe-wk2 -> wk2 -> wpe -> generic

Which makes sense for macOS/iOS, but not for us. We need to override it to make it this:

wpe -> wk2 -> generic

matching the GTK port.

[Zan Dobersek]

(In reply to Michael Catanzaro from comment #3)
> Comment on attachment 314474 [details]
> Patch
> 
> I don't think we should do this.

(In reply to Carlos Alberto Lopez Perez from comment #5)
> I'm also wondering why WPE should not inherit the WK2 expectations ??

As the expectations for resource load statistics tests showed, these TestExpectations files degenerated a bit, and the more TestExpectations files a port relies on, the more problems we'll have.

[Zan Dobersek]

(In reply to Michael Catanzaro from comment #9)
> (In reply to Carlos Alberto Lopez Perez from comment #5)
> > I'm also wondering why WPE should not inherit the WK2 expectations ??
> 
> The problem is the default expectations path is this:
> 
> wpe-wk2 -> wk2 -> wpe -> generic
> 
> Which makes sense for macOS/iOS, but not for us. We need to override it to
> make it this:
> 
> wpe -> wk2 -> generic
> 
> matching the GTK port.

Let's use this bug to do that instead.

[Zan Dobersek]

Created attachment 314963 [details]
Patch

[Michael Catanzaro]

Comment on attachment 314963 [details]
Patch

The code is gobbledigook, but it looks similarish to the GTK port code and I presume it does what you say. Thanks!

But fix the duplicate ChangeLog entry!

[Zan Dobersek]

Created attachment 314976 [details]
Patch for landing

Now also includes webkitpy unit tests.

[Michael Catanzaro]

Comment on attachment 314976 [details]
Patch for landing

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

> Tools/Scripts/webkitpy/port/wpe.py:95
> +        return 2

Hm interesting, since it's not what the GTK unittest does....

[Build Bot]

Comment on attachment 314976 [details]
Patch for landing

Attachment 314976 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4091817

New failing tests:
http/tests/loading/resourceLoadStatistics/user-interaction-in-cross-origin-sub-frame.html

[Build Bot]

Created attachment 314988 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Zan Dobersek]

Comment on attachment 314976 [details]
Patch for landing

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

>> Tools/Scripts/webkitpy/port/wpe.py:95
>> +        return 2
> 
> Hm interesting, since it's not what the GTK unittest does....

Not sure what you're pointing out exactly, but the GTKPort class has the exact same override, for the same reason (hence it's where I copied it from :>):
https://trac.webkit.org/browser/webkit/trunk/Tools/Scripts/webkitpy/port/gtk.py#L225

[Zan Dobersek]

Comment on attachment 314976 [details]
Patch for landing

Clearing flags on attachment: 314976

Committed r219300: <http://trac.webkit.org/changeset/219300>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.