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

Fix test bundling on physical iOS devices #1303

Merged
merged 11 commits into from
May 23, 2023

Conversation

bartekpacia
Copy link
Contributor

@bartekpacia bartekpacia commented May 22, 2023

This PR fixes #1297.

Caveats

  • Running tests on iOS works only in release mode (patrol test --release).

  • Hot Restart doesn't work.
    That's because, in a Flutter app built in release mode, there's no Dart VM that can JIT compile code.
    In other words, for Hot Restart to work, the app must be started in debug mode on iOS.
    Starting the app in debug mode on iOS is mutually exclusive with test bundling.

    A workaround would be to not use test bundling at all when using Hot Restart.
    This would require re-adding the code that was present before test bundling.

    But even if we fixed this and re-added that, Hot Restart on iOS wouldn't work right away. We'd also have to fix Hot restart on physical iOS is not working #1059.

  • Enabling or disabling airplane mode crashes the test suite.

cc: @mateuszwojtczak, @jBorkowska

@docs-page
Copy link

docs-page bot commented May 22, 2023

To view this pull requests documentation preview, visit the following URL:

patrol.leancode.co/~1303

Documentation is deployed and generated using docs.page.

@bartekpacia bartekpacia changed the base branch from master to develop May 22, 2023 21:08
@github-actions github-actions bot added the package: patrol Related to the patrol package (native automation, test bundling) label May 22, 2023
@bartekpacia
Copy link
Contributor Author

Tested locally on my private iPhone and iPad and it works. Here's the xcresult:

Screenshot 2023-05-23 at 12 01 28 AM

@github-actions github-actions bot added the package: patrol_cli Related to the patrol_cli package label May 22, 2023
@@ -140,12 +136,6 @@ class IOSTestBackend {
final subject = '${options.description} on ${device.description}';
final task = _logger.task('Running $subject');

Process? iosDeployProcess;
Copy link
Contributor Author

@bartekpacia bartekpacia May 22, 2023

Choose a reason for hiding this comment

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

Note:

Calling XCUIApplication.launch() with the debugger attached causes really nasty crashes.

@bartekpacia
Copy link
Contributor Author

bartekpacia commented May 22, 2023

TODO:

Print error message when trying to patrol test on physical iOS device.

For example:

Running tests on a physical iOS devices requires the --release flag.

@bartekpacia bartekpacia force-pushed the fix/bundling_physical_ios branch from bf896a4 to ca57328 Compare May 23, 2023 08:04
@bartekpacia bartekpacia requested a review from shilangyu May 23, 2023 08:16
@bartekpacia
Copy link
Contributor Author

A workaround would be to not use test bundling at all when using Hot Restart.
This would require re-adding the code that was present before test bundling.

Actually, we're already doing this.

But still, the Flutter bug breaks Hot Restart on physical iOS devices.

@shilangyu
Copy link
Contributor

Are these limitations documented anywhere (do you have any changelog?)?

* Running tests on iOS works only in release mode (`patrol test --release`).

What is the predicted impact of this limitation? Is there any benefit of debug mode testing?

* Enabling or disabling airplane mode crashes the test suite.

Because of some connection issue?

Copy link
Contributor

@mateuszwojtczak mateuszwojtczak left a comment

Choose a reason for hiding this comment

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

LGTM! Do we plan using go-ios?

@bartekpacia
Copy link
Contributor Author

Do we plan using go-ios?

go-ios is a replacement for ios-deploy. And since in this PR I've removed our usage of ios-deploy, using go-ios doesn't make much sense anymore.

@bartekpacia
Copy link
Contributor Author

bartekpacia commented May 23, 2023

@shilangyu

Are these limitations documented anywhere (do you have any changelog?)?

Yes, we do, although it's pretty basic (https://pub.dev/packages/patrol/changelog).

We'll be rewriting the docs for 2.0 + writing a migration guide.

What is the predicted impact of this limitation? Is there any benefit of debug mode testing?

  • no debugging on physical iOS devices, including:
    • no asserts in Dart
    • flutter logs doesn't work
  • longer builds

We're aware of this.

Enabling or disabling airplane mode crashes the test suite.

Because of some connection issue?

I don't know, but I suspect it's because Patrol (running in the app under test) makes HTTP requests to the Patrol server (running in the instrumentation app). More specifically, all communication between these 2 apps happen over gRPC (which builds on HTTP/2’s long-lived connections).

And toggling (yes, toggling - everything works when airplane mode is enabled, and disabling it results in tests crashing) airplane mode probably somehow interferes with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: patrol_cli Related to the patrol_cli package package: patrol Related to the patrol package (native automation, test bundling)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants