-
Notifications
You must be signed in to change notification settings - Fork 109
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
Run build-artifacts from Ubuntu and Windows in the emulator #169
Conversation
9c419a3
to
c42aa9b
Compare
7d62136
to
a15e659
Compare
a15e659
to
93ea00a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
@dvc94ch immediately resolving your comments: I intentionally pushed a revert of @msiglreith's fix to showcase that this improved Emulator setup indeed catches that issue :) |
Without having to download the artifact. Output is limited to errors and app-specific output anyway.
577305d
to
19ce285
Compare
looks good now |
Now that we have a failure case clearly visible and linked: https://github.com/rust-windowing/android-ndk-rs/runs/3274663232?check_suite_focus=true this PR is now running with the fix in place and should succeed. Ready for review and merge :) |
19ce285
to
03aa316
Compare
The script strangely has a tendency to get stuck, on a different
A working build looks like:
|
Ehh, let's try one more time...? Is this something preventing us from running >2 emulators in a single CI run? None of these errors appear related to our scripts at all. EDIT: This was the run on my local fork (because we have |
For #162 (comment)
TODO: We're not testingThis is now happening in a thirdcargo apk run
functionality anymore, how do we want to work that into here? We can place the commands insiderust.yml
in a third emulator run.local
run.Note that this PR is intentionally not based on the merge of #168 to demonstrate it actually failing! This would have caught the bogus refactor in #162.