[webkit-reviews] review denied: [Bug 215543] Migrate some iOS test expectations from Apple's internal tree : [Attachment 406673] Patch

Sat Aug 15 20:20:17 PDT 2020

Alexey Proskuryakov <ap at webkit.org> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 215543: Migrate some iOS test expectations from Apple's internal tree
https://bugs.webkit.org/show_bug.cgi?id=215543

Attachment 406673: Patch

https://bugs.webkit.org/attachment.cgi?id=406673&action=review

--- Comment #3 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 406673
  --> https://bugs.webkit.org/attachment.cgi?id=406673
Patch

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

This moves partly obsolete iOS 14 expectations on top of iOS 13 expectations.
This should not be done mechanically, as this is the time where we verify which
ones got fixed throughout the development cycle (see some examples below).

But if done mechanically, LayoutTests/platform/ios-13 should be updated to not
lose test coverage in the meanwhile.

> LayoutTests/platform/ios-device/TestExpectations:143
> +fast/events/ios/rotation [ Skip ] #
https://bugs.webkit.org/show_bug.cgi?id=175800

All fast/events/ios tests are skipped in open source, so moving this line from
internal does not make sense.

> LayoutTests/platform/ios-device/TestExpectations:148
> +# <rdar://problem/42138622> [iOS] WebGL conformance LayoutTests failing when
run on device

I wonder if this is still the case, now that we have IOSurface in simulator.

> LayoutTests/platform/ios-simulator-wk2/TestExpectations:134
> +# <rdar://problem/51449034>

This radar was fixed a long time ago.

> LayoutTests/platform/ios-simulator-wk2/TestExpectations:135
> +[ Debug ] media/W3C/audio/events/event_progress.html [ Pass Failure ]

But this test used to be flaky on some old internal builds of iOS 14. Not any
more though, and never in open source in recent memory.

> LayoutTests/platform/ios-simulator-wk2/TestExpectations:136
> +[ Debug ] media/W3C/video/events/event_order_loadstart_progress.html [ Pass
Failure ]

This test didn't fail in recent memory.

> LayoutTests/platform/ios-simulator-wk2/TestExpectations:139
> +# <rdar://problem/60227623> [ wk1 and wk2 ]
fast/text/international/generic-font-family-language-traditional.html is
failing.
> +fast/text/international/generic-font-family-language-traditional.html [
ImageOnlyFailure ]

This test isn't failing, and the radar is fixed.

> LayoutTests/platform/ios-simulator-wk2/TestExpectations:142
> +# <rdar://problem/61889653> [ wk2 Guard-Malloc ]
fast/block/nested-renderers.html is flaky crashing with alert -
WebCore::FrameLoader::commitProvisionalLoad() + 5563
> +fast/block/nested-renderers.html [ Pass Crash ]

This was a one-time memory corruption crash outside WebKit, we shouldn't mark
the test as a flaky crash because of that.

> LayoutTests/platform/ios-simulator-wk2/TestExpectations:145
> +# <rdar://problem/22064820> Test
compositing/backing/form-controls-backing.html failing on iOS Simulator since
it was added
> +compositing/backing/form-controls-backing.html [ Failure Timeout ]

There weren't any timeouts in recent memory. There is already a Failure
expectation for this test LayoutTests/platform/ios/TestExpectations, adding
another one in this file is not helpful.

> LayoutTests/platform/ios-simulator-wk2/TestExpectations:149
> +# <rdar://problem/63539061> REGRESSION: (r261506?): [ iOS wk2 ]
http/tests/in-app-browser-privacy/app-bound-domain-gets-app-bound-session.html
is failing consistently.
>
+http/tests/in-app-browser-privacy/app-bound-domain-gets-app-bound-session.html
[ Failure ]
>
+http/tests/in-app-browser-privacy/non-app-bound-iframe-under-app-bound-domain-
is-app-bound.html [ Failure ]

This radar was fixed in June.

> LayoutTests/platform/ios-simulator-wk2/TestExpectations:152
> +# <rdar://66598268> ([ wk2 ] quicklook/powerpoint-legacy.html is a constant
failure)
> +quicklook/powerpoint-legacy.html [ Pass Failure ]

This is an underlying framework bug in iOS 14, we should not add expectations
for it in open source and reduce iOS 13 coverage now.

> LayoutTests/platform/ios-simulator-wk2/TestExpectations:155
> +# rdar://67034553 ([ Layout Tests ] REGRESSION (r295093?): [ wk2 ]
animations/unprefixed-events-mixed-with-prefixed.html is a flaky failure)
> +animations/unprefixed-events-mixed-with-prefixed.html

Not really sure what's going on with this once, but it's also claimed to be an
iOS 14 issue.

> LayoutTests/platform/ios-simulator-wk2/TestExpectations:158
> +# rdar://67079948 ([ Layout Tests ] REGRESSION (r265515): [ wk2 ]
fast/events/touch/ios/touch-events-with-modifiers.html is a constant failure)
> +fast/events/touch/ios/touch-events-with-modifiers.html [ Pass Failure ]

This test shouldn't be running in open source at all, as touch events
implementation is different. The whole fast/events/touch/ios directory is
skipped in open source, so this incorrectly unskips the test.

Like for all internal only tests, this test's expectation should not be mixed
with open source expectations.

> LayoutTests/platform/ios-simulator/TestExpectations:114
> +# <rdar://problem/35756253> REGRESSION (17E105): 8 canvas test failures
> +canvas/philip/tests/2d.shadow.image.scale.html [ Failure ]

This radar was fixed in 2018, and at least this first test is passing on iOS
Simulator, didn't check others in this group.

> LayoutTests/platform/ios-simulator/TestExpectations:122
> +# rdar://52615537
(UIHelper.setKeyboardInputModeIdentifier("zh_Hans-Pinyin at sw=Pinyin-Simplified;h
w=Automatic") does not switch to Pinyin unless sim configured with Pinyin
(199472))
>
+fast/events/ios/keydown-keyup-keypress-keys-in-non-editable-using-chinese-keyb
oard.html [ Skip ]

All fast/events/ios tests are skipped in open source, so moving the expectation
from internal does not make sense.

>> LayoutTests/platform/ios-simulator/TestExpectations:124
>> +# This test requires force input and is skiped in the general case.
> 
> Probably should fix the spelling of "skipped", but also, what does this
comment mean? The whole directory is skipped for non-iOS, and it seems we
should un-skip the whole directory, not a single file. I don’t think we need to
say "requires force input"; these tests are in a directory named iOS.

Most of the tests are run using iPhone SE simulator, where pressure events
don't work AFAIK. There are select few that run using iPhone 7 simulator.

I think that the internal expectations are confused. The test was made iPhone 7
only in rdar://53458890, but then the expectation was moved into the general
expectations file in an "unreviewed gardening" patch.