Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[snapshot] wait for simulator to boot before overriding status bar #19344

Merged
merged 1 commit into from Sep 15, 2021

Conversation

JosephDuffy
Copy link
Contributor

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Motivation and Context

If the simulator is not booted when the status bar is updated it does not do anything and the default status bar is used.

Resolves #19317

Description

xcrun instruments fails with the error "xcrun: error: Failed to locate 'instruments'." This command will boot the simulator if it's not curerntly booted and then wait for it finish booting before terminating.

Testing Steps

Use snapshot while the override_status_bar option is true.

Some of the tests failed for me, although they do not seem related to this change.
rspec ./fastlane/spec/actions_specs/app_store_connect_api_key_spec.rb:95 # Fastlane Fastlane::FastFile App Store Connect API Key raise error when no key_filepath or key_content
rspec ./fastlane/spec/actions_specs/import_from_git_spec.rb:162 # Fastlane Fastlane::FastFile import_from_git with caching works with new tags
rspec ./fastlane/spec/actions_specs/import_from_git_spec.rb:200 # Fastlane Fastlane::FastFile import_from_git with caching works with branch
rspec ./match/spec/importer_spec.rb:47 # Match Match::Runner imports a .cert, .p12 and .mobileprovision (iOS provision) into the match repo
rspec ./match/spec/importer_spec.rb:64 # Match Match::Runner imports a .cert, .p12 and .provisionprofile (osx provision) into the match repo
rspec ./match/spec/importer_spec.rb:81 # Match Match::Runner imports a .cert and .p12 without profile into the match repo (backwards compatibility)
rspec ./match/spec/importer_spec.rb:98 # Match Match::Runner imports a .cert and .p12 when the type is set to developer_id
rspec ./pilot/spec/build_manager_spec.rb:661 # Build Manager #transporter_for_selected_team with one team id
rspec ./pilot/spec/build_manager_spec.rb:677 # Build Manager #transporter_for_selected_team with inferred provider id
rspec ./pilot/spec/build_manager_spec.rb:633 # Build Manager #transporter_for_selected_team with itc_provider with nil Spaceship::TunesClient
rspec ./pilot/spec/build_manager_spec.rb:646 # Build Manager #transporter_for_selected_team with itc_provider with nil Spaceship::TunesClient
rspec ./pilot/spec/manager_spec.rb[1:1:2:2:1:1:1] # Pilot Pilot::Manager what happens on 'login' when using web session when username input param is given behaves like performing the spaceship login using username and password by pilot performs the login using username and password
rspec ./pilot/spec/manager_spec.rb[1:1:2:2:2:1:1] # Pilot Pilot::Manager what happens on 'login' when using web session when username input param is not given but found apple_id in AppFile behaves like performing the spaceship login using username and password by pilot performs the login using username and password
rspec ./sigh/spec/runner_spec.rb:142 # Sigh Sigh::Runner#devices_to_use devices for development
rspec ./sigh/spec/runner_spec.rb:152 # Sigh Sigh::Runner#devices_to_use devices for adhoc

`xcrun instruments` fails with the error "xcrun: error: Failed to locate 'instruments'." This command will boot the simulator if it's not curerntly booted and then wait for it finish booting before terminating.

Fixes fastlane#19317.
@google-cla google-cla bot added the cla: yes label Sep 9, 2021
@joshdholtz joshdholtz changed the title Wait for simulator to boot before overriding status bar [snapshot] wait for simulator to boot before overriding status bar Sep 14, 2021
Copy link
Member

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a quick question on the difference between this two subcommands 🙃 This would be ag great fix but just a little bit curious at what this change actually does 😛

Comment on lines -119 to +120
Helper.backticks("xcrun instruments -w #{device_udid} &> /dev/null")
# Boot the simulator and wait for it to finish booting
Helper.backticks("xcrun simctl bootstatus #{device_udid} -b &> /dev/null")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you happen to have any good documentation or explanation on what the difference between these two commands are? I tried to look at the help/man pages and couldn't really find too much h🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Xcode 12.5.1 as the current Xcode xcrun instruments outputs:

`instruments` is now deprecated in favor of 'xcrun xctrace' (see `man xctrace` for more information on its replacement)
instruments, version 12.5.1 (64544.132)
usage: instruments [-t template] [-D document] [-l timeLimit] [-w device] [-P package] [[-p pid] | [application [-e variable value] [argument ...]]]

man instruments describes -w as:

     -w [hardware device, or simulator name]
              The name or identifier of the hardware device or simulator to target. Should be specified first

(you probably found this documentation but I'm adding it for completeness)

The behaviour has either changed or this never worked because using instruments to boot the simulator only boots the simulator (it doesn't wait for it to be ready) and exits with an error:

instruments[2650]: Waiting for device to boot...
Instruments Usage Error: No template (-t) specified
instruments, version 12.5.1 (64544.132)
usage: instruments [-t template] [-D document] [-l timeLimit] [-w device] [-P package] [[-p pid] | [application [-e variable value] [argument ...]]]

bootstatus is not listed in xcrun simctl help but does have help text available via xcrun simctl help bootstatus:

Checks device boot status.
Usage: simctl bootstatus <device> [-bcd]
	-b       Boot the device if it isn't already booted.
	-d       Print data migration information, if it's available.
	-c       Continuously monitor boot status through multiple boot/shutdown cycles.

Monitors the specified device and prints boot status information until the device finishes booting. You can safely call this before you attempt to start booting the device.

I found this quite confusing myself.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my goodness! The xcrun simctl help bootstatus is the command I need to find but couldn't 😓 Thank you so much for this! It does appear that this never really worked then 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit hidden! 😔 I would love to know if there are any other (hidden) commands available but this is the only one I'm aware of.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this tipp! This saved me from HOURS of debugging in my screenshotting tool!

@joshdholtz
Copy link
Member

@JosephDuffy Quick question for you! Is your status bar actually changing like you are expecting it to in your screenshots? I actually can't get this to work for me 😬 What are the options you are using with snapshot that you are getting this to work?

@joshdholtz
Copy link
Member

@JosephDuffy Turns out I was an idiot and somehow deleted my snapshot() call from my UI test without knowing 🤦‍♂️

Copy link
Member

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix is amazing and seems to fix the issue happening on this discussion 👉 #17021

Thank you so much for this! Really appreciate it ❤️

@joshdholtz joshdholtz merged commit 6bcd74c into fastlane:master Sep 15, 2021
@fastlane-bot
Copy link

Hey @JosephDuffy 👋

Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉
The code change now lives in the master branch, however it wasn't released to RubyGems yet.
We usually ship about once a week, and your PR will be included in the next one.

Please let us know if this change requires an immediate release by adding a comment here 👍
We'll notify you once we shipped a new release with your changes 🚀

Copy link

@fastlane-bot fastlane-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congratulations! 🎉 This was released as part of fastlane 2.194.0 🚀

@JosephDuffy
Copy link
Contributor Author

Thanks for the quick review and merge Josh!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Snapshot: override_status_bar not working
4 participants