Fix a AppLifecycleTest present time race between the animation and the rest of the test #21107

xster · 2020-09-11T17:55:49Z

This PR fixes the following:

Re-enable tests after flutter/flutter#61620

There was previously no gate on the no animation vc present. There was a chance that more code could concurrently run between viewWillAppear and viewDidAppear which produces indeterministic behavior.

There was also a chance that AppLifecycleTest is the first test to run. When that happens, there's a chance that UIApplication's application state is UIApplicationStateInactive (couldn't repro locally but happens on LUCI). The UIApplicationStateInactive->UIApplicationStateActive transition could happen in the test which is problematic since AppLifecycleTest asserts all lifecycle transitions.

I wasn't able to force it to transition first during setup and wait, so I just accounted for this possible extra transition in the test.

The special casing seems to have worked in https://logs.chromium.org/logs/flutter/led/xster_google.com/18a0dadc0f7e51e15dbbbe4666b2392ad20e6226b1050c54435b83902e90f2bf/+/steps/Scenario_App_Integration_Tests/0/stdout, https://logs.chromium.org/logs/flutter/led/xster_google.com/3449f1df356e5fce74d34bb2e783ccd9f2eba95b9911a0f23357162a096376a9/+/steps/Scenario_App_Integration_Tests/0/stdout, https://logs.chromium.org/logs/flutter/led/xster_google.com/a6de2ad7906874242d6b596eaf1431e9ac72cb61640045744ce14af7b614d3d4/+/steps/Scenario_App_Integration_Tests/0/stdout.

Try 38681f4:
https://chromium-swarm.appspot.com/task?id=4e9721417a406210
https://chromium-swarm.appspot.com/task?id=4e97216ddf9d1b10
https://chromium-swarm.appspot.com/task?id=4e9721ef95a11710
https://chromium-swarm.appspot.com/task?id=4e972212b205be10
https://chromium-swarm.appspot.com/task?id=4e97223052766810
https://chromium-swarm.appspot.com/task?id=4e97225071f95110

Try fb75e54:
https://chromium-swarm.appspot.com/task?id=4e97642f08353010
https://chromium-swarm.appspot.com/task?id=4e97643887800f10
https://chromium-swarm.appspot.com/task?id=4e97644154c88f10
https://chromium-swarm.appspot.com/task?id=4e97644b716a2510
https://chromium-swarm.appspot.com/task?id=4e976455d4d18810
https://chromium-swarm.appspot.com/task?id=4e97645f1c875810
https://chromium-swarm.appspot.com/task?id=4e9764660d917a10
https://chromium-swarm.appspot.com/task?id=4e97646e65d20c10

Try bebce3d:
https://chromium-swarm.appspot.com/task?id=4e97f4f8ca6e5210
https://chromium-swarm.appspot.com/task?id=4e97f4ff341ef010
https://chromium-swarm.appspot.com/task?id=4e97f50e72eb1510
https://chromium-swarm.appspot.com/task?id=4e97f51dc07e3d10
https://chromium-swarm.appspot.com/task?id=4e97f5255604d310
https://chromium-swarm.appspot.com/task?id=4e97f52e46ef2d10
https://chromium-swarm.appspot.com/task?id=4e97f53a22042110
https://chromium-swarm.appspot.com/task?id=4e97f541f218bd10

Try c28013a:
https://chromium-swarm.appspot.com/task?id=4e980e9a88ca8410
https://chromium-swarm.appspot.com/task?id=4e980ea26762a510
https://chromium-swarm.appspot.com/task?id=4e980eaa990d1010
https://chromium-swarm.appspot.com/task?id=4e980eb303239210
https://chromium-swarm.appspot.com/task?id=4e980ebb546b0f10
https://chromium-swarm.appspot.com/task?id=4e980ec444f57810
https://chromium-swarm.appspot.com/task?id=4e980eceaf85ad10
https://chromium-swarm.appspot.com/task?id=4e980ed81654d010

flutter-dashboard · 2020-09-11T17:55:51Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

fluttergithubbot · 2020-09-11T20:55:01Z

This pull request is not suitable for automatic merging in its current state.

The status or check suite Mac iOS Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

gaaclarke · 2020-09-11T22:45:00Z

testing/scenario_app/ios/Scenarios/Scenarios/ScreenBeforeFlutter.m

@@ -39,11 +39,11 @@ - (void)viewDidLoad {
  [_engine runWithEntrypoint:nil];
 }

- (FlutterViewController*)showFlutter {
+- (FlutterViewController*)showFlutter:(void (^)(void))showCompletion {


fyi you can just say dispatch_block_t

xster · 2020-09-11T23:40:13Z

Last try no good. When AppLifecycleTest is the first scenario test to run, the application state is in UIApplicationStateInactive. Can't just start to test since if the UIApplicationStateInactive->UIApplicationStateActive transition happens in the test, it'll pollute. Can't just wait since sometimes it never starts without more actions in the test.

Probably need to start a dummy UIViewController first to poke at it if needed.

…idAppear

xster · 2020-09-12T07:29:47Z

k, I think I finally got this working in the last 12/12 (with 3 hitting the previously problematic sequence).

I'll submit this Monday morning so it doesn't crap out when I'm out.

xster · 2020-09-14T15:50:12Z

Merging to kick purple bot

…n and the rest of the test (flutter/engine#21107)

flutter-dashboard bot added the platform-ios label Sep 11, 2020

googlebot added cla: yes labels Sep 11, 2020

xster marked this pull request as draft September 11, 2020 17:56

auto-assign bot requested a review from iskakaushik September 11, 2020 17:56

xster removed the request for review from iskakaushik September 11, 2020 17:57

xster marked this pull request as ready for review September 11, 2020 19:50

xster requested review from cbracken and gaaclarke September 11, 2020 19:51

xster changed the title ~~Debug AppLifecycleTest~~ Fix a AppLifecycleTest present time race between the animation and the rest of the test Sep 11, 2020

cbracken approved these changes Sep 11, 2020

View reviewed changes

xster added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Sep 11, 2020

fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Sep 11, 2020

gaaclarke reviewed Sep 11, 2020

View reviewed changes

xster mentioned this pull request Sep 11, 2020

Disable iOS application lifetime Scenario tests #21125

Merged

xster added 9 commits September 11, 2020 17:19

Debug AppLifecycleTest

d89830a

more debug

5392fed

fix present time race that can occur between viewWillAppear and viewD…

4593609

…idAppear

clean up

9566df4

wait for application active

f21361b

s/void (^)(void)/dispatch_block_t

df4f3cb

give more time for simulator to start

5e7ebbd

move the application state wait until after setting the root vc

63759ca

unskip

624f2fc

xster force-pushed the debug-applifecycletest branch from 7c6573c to 624f2fc Compare September 12, 2020 00:20

cbracken mentioned this pull request Sep 12, 2020

[engine] Scenario tests are flaky on flutter-try-mac-x machines flutter/flutter#61620

Closed

try with keyValueObservingExpectationForObject instead

334e1db

xster added 2 commits September 11, 2020 23:57

just account for the extra lifecycle if first run ever

61e8b35

autoformat

0832ad6

xster added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Sep 14, 2020

xster merged commit 257eba9 into flutter:master Sep 14, 2020

xster deleted the debug-applifecycletest branch September 14, 2020 15:50

engine-flutter-autoroll mentioned this pull request Sep 14, 2020

Roll Engine from 2fc054147e57 to 257eba9c19b5 (3 revisions) flutter/flutter#65769

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 14, 2020

257eba9 Fix a AppLifecycleTest present time race between the animatio…

bc15d93

…n and the rest of the test (flutter/engine#21107)

cbracken mentioned this pull request Sep 14, 2020

Re-disable iOS Scenarios tests due to flakiness #21123

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix a AppLifecycleTest present time race between the animation and the rest of the test #21107

Fix a AppLifecycleTest present time race between the animation and the rest of the test #21107

xster commented Sep 11, 2020 •

edited

Loading

flutter-dashboard bot commented Sep 11, 2020

fluttergithubbot commented Sep 11, 2020

gaaclarke Sep 11, 2020

xster commented Sep 11, 2020

xster commented Sep 12, 2020

xster commented Sep 14, 2020

Fix a AppLifecycleTest present time race between the animation and the rest of the test #21107

Fix a AppLifecycleTest present time race between the animation and the rest of the test #21107

Conversation

xster commented Sep 11, 2020 • edited Loading

flutter-dashboard bot commented Sep 11, 2020

fluttergithubbot commented Sep 11, 2020

gaaclarke Sep 11, 2020

Choose a reason for hiding this comment

xster commented Sep 11, 2020

xster commented Sep 12, 2020

xster commented Sep 14, 2020

xster commented Sep 11, 2020 •

edited

Loading