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

Auto-restart xctest runner and idb_companion on connection error #882

Merged
merged 4 commits into from
Mar 13, 2023

Conversation

berikv
Copy link
Contributor

@berikv berikv commented Mar 9, 2023

Proposed Changes

When a connection error occurs on either xctest runner or on idb_companion, restart that specific service.

Testing

Early testing looks promising, more testing is needed.
Tested by:

  • Adjusting LocalIOSDevice.tap to use IdbIOSDevice.tap
  • Running ./maestro studio in one terminal
  • Running ./maestro test samples/ios-flow.yaml several times in another terminal
  • Executing a tapOn command in the still open studio

Issues Fixed

launchApp would cause idb_companion to shut down due to its internal setPermissions call. idb_companion is needed as a fallback for getViewHierarchy for ReactNative (and other apps with big view hierarchies).

@berikv berikv force-pushed the xctestrunner_restart_onerror branch from 9fba62c to 9c42868 Compare March 9, 2023 16:59
@berikv berikv changed the title Auto-restart xctest runner on connection error Auto-restart xctest runner and idb_companion on connection error Mar 12, 2023
@berikv berikv marked this pull request as ready for review March 12, 2023 14:57
@berikv berikv force-pushed the xctestrunner_restart_onerror branch from 48b8f0d to e879513 Compare March 12, 2023 15:06
Copy link
Collaborator

@dmitry-zaitsev dmitry-zaitsev left a comment

Choose a reason for hiding this comment

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

Let's make sure to somehow inform the user on why their device is being rebooted, otherwise it can get very confusing

@berikv
Copy link
Contributor Author

berikv commented Mar 13, 2023

Let's make sure to somehow inform the user on why their device is being rebooted, otherwise it can get very confusing

Devices are not rebooted, only idb is restarted

@artem888
Copy link
Contributor

artem888 commented Mar 13, 2023

Let's make sure to somehow inform the user on why their device is being rebooted, otherwise it can get very confusing

Devices are not rebooted, only idb is restarted

seems that applesimutils causes some kinda device reboot, but I don't see an easy workaround there https://mobile-dev-inc.slack.com/archives/C041FU72T54/p1678455454132919?thread_ts=1678452522.490319&cid=C041FU72T54

@berikv
Copy link
Contributor Author

berikv commented Mar 13, 2023

Let's make sure to somehow inform the user on why their device is being rebooted, otherwise it can get very confusing

Devices are not rebooted, only idb is restarted

seems that applesimutils causes some kinda device reboot, but I don't see an easy workaround there https://mobile-dev-inc.slack.com/archives/C041FU72T54/p1678455454132919?thread_ts=1678452522.490319&cid=C041FU72T54

It looks more like a UI refresh, applesimutils probably kills some simulator os-level gui process to make its changes persistent.

Do we really want to warn the user about that though? And through which channel? We currently don't have a very simple way to reliable warn the user about something. Maestro's ansi UI overwrites any println() that we do. I could trigger a system alert, but I don't think we'd make friends that way..

@berikv berikv force-pushed the xctestrunner_restart_onerror branch from dd72b2d to 6e97409 Compare March 13, 2023 13:58
object IdbCompanion {
private val logger = DebugLogStore.loggerFor(IdbCompanion::class.java)

// TODO: Understand why this is a separate method from strartIdbCompanion
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amanjeetsingh150 Do you know the details on why this was implemented differently than startIdbCompanion?

@berikv berikv merged commit 1a65d19 into main Mar 13, 2023
@berikv berikv deleted the xctestrunner_restart_onerror branch March 13, 2023 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants