Create iOS simulator before running IosUnitTests #33906

jmagman · 2022-06-08T17:38:31Z

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

gaaclarke · 2022-06-08T23:10:50Z

testing/run_tests.py

+      'xcrun '
+      'simctl '
+      'create '
+      '%s com.apple.CoreSimulator.SimDeviceType.iPhone-11' % new_simulator_name


If this line of code changes 634 has to change too. Can the test command optionally take in the simulator name for the destination? Otherwise at a minimum we can pull these out into variables that are next to each other so it's clear they are updated in tandem.

Actually, I think we can get away with changing the 634 logic from platform='iOS Simulator,name=iPhone 11' to name=IosUnitTestsSimulator. I forgot that was why I needed to delete the old simulators so there were never multiple with the same name, or the xcodebuild command would fail since there were multiple possible destinations. Let me do that.

(obligatory comment to make this a request)

gaaclarke · 2022-06-08T23:11:23Z

testing/run_tests.py

+      test_command[0] = test_command[0] + " -only-testing:%s" % test_filter
+    RunCmd(test_command, cwd=ios_unit_test_dir, shell=True)
+  finally:
+    DeleteSimulator(new_simulator_name)


This isn't technically needed to fix the issue and it will increase run times for tests no?

You're right, this is technically not needed since any leftover simulators are deleted before the next run of this script (which happens during flake detection), but I'd rather not leak the simulator to other scripts running simulator tests (scenario?)
Fortunately it's very fast, as mentioned above:

xcrun simctl delete test1 0.08s user 0.08s system 86% cpu 0.180 total

cyanglaz · 2022-06-08T23:37:41Z

testing/run_tests.py

-      'FLUTTER_ENGINE=' + ios_variant
+  # Delete simulators with this name in case any were leaked
+  # from another test run.
+  DeleteSimulator(new_simulator_name)


Another option is to detect if the simulator exists and decide to create a new one if one does not exist.

That seemed like a lot more work and parsing 🙂

I wanted to guard against the simulator leaking when this script is run multiple times for flake detection. Fortunately creating and deleting simulators is extremely fast, so we don't need to over optimize.

xcrun simctl create test1 com.apple.CoreSimulator.SimDeviceType.iPhone-11 0.07s user 0.07s system 65% cpu 0.217 total

xcrun simctl delete test1 0.08s user 0.08s system 86% cpu 0.180 total

cyanglaz

LGTM!

chinmaygarde · 2022-06-09T20:09:13Z

Can we land this?

gaaclarke · 2022-06-09T20:15:13Z

Can we land this?

I was waiting on the change to using the name instead of the platform to launch the simulator. Now that the lights are green, does that mean it worked? If that's the case I'm good with it.

gaaclarke · 2022-06-09T20:15:49Z

testing/run_tests.py

+      'xcrun '
+      'simctl '
+      'create '
+      '%s com.apple.CoreSimulator.SimDeviceType.iPhone-11' % new_simulator_name


(obligatory comment to make this a request)

jmagman · 2022-06-09T22:05:26Z

I was waiting on the change to using the name instead of the platform to launch the simulator. Now that the lights are green, does that mean it worked? If that's the case I'm good with it.

Yup, it worked.

Running command "xcrun simctl create IosUnitTestsSimulator com.apple.CoreSimulator.SimDeviceType.iPhone-11"
No runtime specified, using 'iOS 15.0 (15.0 - 19A339) - com.apple.CoreSimulator.SimRuntime.iOS-15-0'
630FEA44-05F6-4AEF-86FF-54C68132CE61

Command run successfully in 0.31 seconds: xcrun simctl create IosUnitTestsSimulator com.apple.CoreSimulator.SimDeviceType.iPhone-11

Running command "xcodebuild -sdk iphonesimulator -scheme IosUnitTests -destination name='IosUnitTestsSimulator' test FLUTTER_ENGINE=ios_debug_sim"

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8811808437021204833/+/u/test:_Host_Tests_for_ios_debug_sim/stdout

gaaclarke

lgtm thanks!

…ine#33906)

jmagman self-assigned this Jun 8, 2022

Create iOS simulator before running IosUnitTests

13cebed

jmagman force-pushed the create-simulator branch from 5a90d40 to 13cebed Compare June 8, 2022 20:21

Whitespace

a55887c

jmagman requested review from cyanglaz and gaaclarke June 8, 2022 22:57

gaaclarke reviewed Jun 8, 2022

View reviewed changes

cyanglaz reviewed Jun 8, 2022

View reviewed changes

cyanglaz approved these changes Jun 9, 2022

View reviewed changes

Try name instead of platform

69001de

gaaclarke suggested changes Jun 9, 2022

View reviewed changes

gaaclarke approved these changes Jun 9, 2022

View reviewed changes

jmagman 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 Jun 9, 2022

fluttergithubbot merged commit 5d89a6c into flutter:main Jun 9, 2022

jmagman deleted the create-simulator branch June 9, 2022 23:57

engine-flutter-autoroll mentioned this pull request Jun 10, 2022

Roll Flutter Engine from 612c8be59d64 to 38c0a2fb5c06 (3 revisions) flutter/flutter#105748

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 10, 2022

5d89a6c Create iOS simulator before running IosUnitTests (flutter/eng…

ac8b142

…ine#33906)

houhuayong pushed a commit to houhuayong/engine that referenced this pull request Jun 21, 2022

Create iOS simulator before running IosUnitTests (flutter#33906)

e73ee63

Create iOS simulator before running IosUnitTests #33906

Create iOS simulator before running IosUnitTests #33906

Uh oh!

Conversation

jmagman commented Jun 8, 2022

Pre-launch Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cyanglaz left a comment

Choose a reason for hiding this comment

Uh oh!

chinmaygarde commented Jun 9, 2022

Uh oh!

gaaclarke commented Jun 9, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmagman commented Jun 9, 2022

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants