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 support for physical iOS devices #181

Merged
merged 36 commits into from
Aug 13, 2022
Merged

Add support for physical iOS devices #181

merged 36 commits into from
Aug 13, 2022

Conversation

bartekpacia
Copy link
Contributor

@bartekpacia bartekpacia commented Aug 8, 2022

The HTTP server library I used for iOS (Embassy) is way too buggy. I'll have to look for alternatives to replace it. It's probably this HTTP library that causes SocketExceptions on the Dart side.

@github-actions github-actions bot added the package: patrol_cli Related to the patrol_cli package label Aug 8, 2022
@bartekpacia
Copy link
Contributor Author

bartekpacia commented Aug 8, 2022

This is WIP. Feel free to look around :)

I'll tag you as reviewers once it's ready, @iasiu @shilangyu.

@bartekpacia bartekpacia force-pushed the feature/ios branch 5 times, most recently from dcb4e49 to a79313b Compare August 11, 2022 20:07
@bartekpacia bartekpacia marked this pull request as ready for review August 12, 2022 09:16
Comment on lines 12 to 14
http GET localhost:8081/isRunning
http POST localhost:8081/openApp id='pl.baftek.discoverRudy'
http POST localhost:8081/pressHome || true
Copy link
Contributor

Choose a reason for hiding this comment

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

Inconsistent indent

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, what is http? Seems like a non-standard tool, if so, it should be somewhere mentioned as a external dependency probably

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 HTTPie, basically a modern curl.

I added a comment to this script.

Comment on lines +88 to 99
// On iOS, "flutter" is not prefixed
final flutterPrefix = RegExp('flutter: ');

// On Android, "flutter" is prefixed with "I\"
final flutterWithPortPrefix = RegExp(r'I\/flutter \(\s*[0-9]+\): ');
if (line.startsWith(flutterWithPortPrefix)) {
log.info(line.replaceFirst(flutterWithPortPrefix, ''));
} else if (line.startsWith(flutterPrefix)) {
log.info(line.replaceFirst(flutterPrefix, ''));
} else {
log.fine(text);
log.fine(line);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

All of these string matching will definitely be a source of bugs :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah :(

Any better ideas?

} else {
p = _defaultArtifactPath;
}

return p;
}

String get _defaultArtifactPath => path.join(_homePath, '.maestro');
String get _defaultArtifactPath => path.join(_homeDirPath, '.maestro');
Copy link
Contributor

Choose a reason for hiding this comment

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

:(

Copy link
Contributor Author

@bartekpacia bartekpacia Aug 12, 2022

Choose a reason for hiding this comment

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

Thanks, I forgot about it. Fixed now.

.github/workflows/maestro_cli-publish.yaml Outdated Show resolved Hide resolved
deviceJson as Map<String, dynamic>;

final targetPlatform = deviceJson['targetPlatform'] as String;
if (targetPlatform != 'android-arm64' && targetPlatform != 'ios') {
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if someone wants to run it on android-x86? or any other architecture supported by android?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, fixed

@bartekpacia bartekpacia requested a review from shilangyu August 13, 2022 13:44
@bartekpacia
Copy link
Contributor Author

Gonna merge to see how (bad) CI behaves.

@bartekpacia bartekpacia merged commit 35e6da4 into master Aug 13, 2022
@bartekpacia bartekpacia deleted the feature/ios branch August 13, 2022 13:45
bartekpacia added a commit that referenced this pull request Sep 15, 2022
* maestro_cli: list also physical iOS devices

* print current IP address

* maestro_cli: add temporary "simulator" flag to `maestro drive`

* run on public interface

* try to handle permission popups

* remove commented-out code

* refactor touching many areas of the codebase, so sad

* fixes
* make `iproxy` work

* disable `swift-format` action

* AutomatorServer/android: minor changes to match iOS server

* create `run_tests` script

* forward $MAESTRO_PORT to Swift

* refactor android AutomatorServer a bit

* maestro_cli: refactor ios_driver code

* maestro_cli: more refactors

* AutomatorServer/ios: fix error catching

* AutomatorServer/ios: fix passing `TEST_RUNNER_MAESTRO_PORT` to Swift code

* maestro_test: remove `package:logging`

* refactor code handling artifacts, create ArtifactsRepository

* big refactor + download iOS artifacts

* maestro_cli: fix iOS artifacts extracting to a wrong place

* fully implement `maestro_cli-publish` workflow for iOS

* remove commented-out code in ServerLoop.swift

* throw Exception when flutter_driver exits with code != 0

* add docs in `ios/run_tests` script

* maestro_cli: change default artifact path to `$HOME/.cache/maestro`
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants