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

Test hello-world example with Android Emulator on CI #133

Merged
merged 1 commit into from
Jul 27, 2021

Conversation

Gordon-F
Copy link
Contributor

No description provided.

Copy link
Contributor

@dvc94ch dvc94ch left a comment

Choose a reason for hiding this comment

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

Neat!

Copy link
Member

@MarijnS95 MarijnS95 left a comment

Choose a reason for hiding this comment

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

Haven't reviewed in detail, but looks cool and simple enough!

.github/workflows/emulator.yml Outdated Show resolved Hide resolved
.github/workflows/android_test.sh Outdated Show resolved Hide resolved
@Gordon-F Gordon-F force-pushed the ci_android_emulator branch from ee580b0 to 457cacf Compare July 26, 2021 17:50
@Gordon-F Gordon-F requested a review from MarijnS95 July 26, 2021 17:51
@Gordon-F Gordon-F force-pushed the ci_android_emulator branch from 457cacf to 1bd4bdc Compare July 27, 2021 10:42
Copy link
Member

@MarijnS95 MarijnS95 left a comment

Choose a reason for hiding this comment

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

Awesome, the test works now! We should probably come up with a more sophisticated test-case at some point, but validating app startup and "hello world" being printed is an important first milestone. Thanks!

@MarijnS95 MarijnS95 merged commit 8021a8a into rust-mobile:master Jul 27, 2021
@Gordon-F Gordon-F deleted the ci_android_emulator branch July 27, 2021 12:02
@MarijnS95
Copy link
Member

@Gordon-F Would you be able to help out? For quite some time now the CI seems to be unresponsive around adb install, timing out the job 3hrs after:

https://github.com/rust-windowing/android-ndk-rs/actions/runs/1452352305

@MarijnS95
Copy link
Member

I vaguely remember writing down some more info in #169, too.

@Gordon-F
Copy link
Contributor Author

@MarijnS95 It looks like an infrastructure problem on the GitHub runner side.

Is macos-latest MacOS BigSur powered runners? Maybe we should switch to macos-15 and run only on Catalina?

@MarijnS95
Copy link
Member

@Gordon-F I bumped it from macos-10.15 to macos-latest in b665be0, and vaguely remember it started happening exactly since that patch. However:

https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions#github-hosted-runners

The macos-latest label currently uses the macOS 10.15 runner image.

So it should not have made a difference?

@Gordon-F
Copy link
Contributor Author

Also, the emulator starts 3 times for 3 different tasks, if I understood everything correctly. Is it necessary? It seems to me that it would be more optimal to start it only 1 time and run all tests on it.

@MarijnS95
Copy link
Member

The broken run also shows that it was on 10.15:

Current runner version: '2.284.0'
Operating System
  Mac OS X
  10.15.7
  19H1419

Also, the emulator starts 3 times for 3 different tasks, if I understood everything correctly. Is it necessary? It seems to me that it would be more optimal to start it only 1 time and run all tests on it.

But we'd like to have a clean emulator for every artifact that we need to test. Could deinstall the application but that seems far from ideal. However, perhaps starting 3 of them instead of one is the problem "somehow" in scheduling?

@Gordon-F
Copy link
Contributor Author

But we'd like to have a clean emulator for every artifact that we need to test.

Android-emulator-runner docs suggest using a clean snapshot to speed up CI in this case. What do you think about it? It should speed up CI time and maybe help with the runner issue.

@MarijnS95
Copy link
Member

Yeah, we can try that :)

@MarijnS95
Copy link
Member

It's a bit strange because this shouldn't happen regardless (I kept the set -x from aforementioned PR #169, and it doesn't show anything out of the ordinary).

@MarijnS95
Copy link
Member

@Gordon-F It also seems to have been fixed by disabling animations, GPU etc from that page, see #198. We can add caching in a separate PR and see how well that speeds things up separate from fixing the regression lockup.

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.

3 participants