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

Add start session command to increase speed of tests #1902

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Almouro
Copy link

@Almouro Almouro commented Aug 9, 2024

Proposed changes

feat:Add start session command to increase speed of tests

When running on lower end Android devices, Maestro can take a long time to start and to finish the test, since it's installing the APKs + uninstalling at the end (can take ~20s on low end devices)

For instance, we use Flashlight to generate e2e performance reports based on several e2e test iterations and we recommend heavily the use of Maestro.
However the time bottleneck of running multiple test iterations is those install/uninstall.

This PR introduces a start-session command:

  • you start it in one tab and it should do the setup necessary, including uninstalling/installing APKs
  • then if you do maestro test in another tab, setup won't be done again, and the tests will be fast ⚡

How it works

I was surprised by how easy it was to do and kudos for that 👏 You guys did a fantastic job there ♥️
The start-session command allows us to keep an active session open. Subsequent test command will find the session active in SessionStore and use it.
Then openDriver will be set to false, and the AndroidDriver won't run the open() function which does all the heavy setup.
Sadly this isn't the case for iOS, so we don't have it working out of the box.

Potentially breaking shards?

⚠️ I had to remove 2 lines of code: I have to be honest, I have to be honest, I didn't manage to understand their purpose 😓. So I'd love some help from @sdfgsdfgd or @igorsmotto to know why they're needed. 🙏

For reference, I've run a "shard" test with ./maestro test -s 2 tests including some launchApp and it worked for me. Likely I've missed a use-case.

Testing

  • Tested locally + in our AWS Device farm CI
  • Also added an e2e test script
Normal behavior With active session ⚡
Screen.Recording.2024-08-09.at.5.24.32.PM.mov
Screen.Recording.2024-08-09.at.5.29.50.PM.mov

Issues fixed

Fixes #1096

Comment on lines -197 to -198
if(!open) // pick device flow, no open() invocation
open()
Copy link
Author

@Almouro Almouro Aug 9, 2024

Choose a reason for hiding this comment

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

As mentioned, I had to remove 2 lines of code: I have to be honest, I didn't manage to understand their purpose. 😓
So I'd love some help from @sdfgsdfgd or @igorsmotto to know why they're needed. 🙏

I've tested maestro test -s 2 tests locally (including some launchApp) and didn't run into any issues, but likely I've missed something.

Here the issue is that since maestro test has found an active session, it didn't "open" the Android driver since it doesn't need to.
But this condition would force it to reopen when using launchApp which defeats the purpose of maestro start-session

@bartekpacia
Copy link
Contributor

bartekpacia commented Aug 9, 2024

What a great pull request @Almouro! 🎉

Thanks a ton! I'll take a look at it the next week.

PS Don't bother with test-cloud failing, it's expected for external contributions.

@bartekpacia
Copy link
Contributor

Do I understand correctly that this is Android-only atm?

@Almouro
Copy link
Author

Almouro commented Aug 12, 2024

@bartekpacia indeed,
probably a bit more work would be needed on iOS, since the iOS driver isn't the one responsible for starting the XCTestInstaller it seems

@bartekpacia
Copy link
Contributor

Thanks for confirming! Feature parity across platforms is very important for us, so we cannot merge it as is.

@bartekpacia bartekpacia marked this pull request as draft August 23, 2024 16:14
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.

Make Maestro faster by giving the option to skip helper APKs install/uninstall
2 participants