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

Explicit run mode and --target for CI tests #63

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ilammy
Copy link
Contributor

@ilammy ilammy commented Jan 1, 2022

This is a demonstration of issue #62.

I'm not really sure what do you want to do with these tests. Right now they are broken and it's not trivial to fix them. Might as well leave them broken as a reminder, or remove them altogether from ci.yml to be re-added when they actually work.

Instead of hacking the mode detection by looking at the OS and target,
just add an explicit variable to control what to do with tests.
When we're testing a target, Cargo needs to be instructed to compile
for that target, not for the host environment it's running in.

Otherwise, all these "arm-android" tests are just building for
"ubuntu-latest" host, which does not really test what it should test.
Once you do that, basically everything that is not compiling for the
host environment fails to run. Leave some FIXMEs there, for now CI
is going to be broken.
I think they belong in the new place, explaining why a particular
test-mode is chosed for a target.
Since they no longer explicitly depend on the target OS, just state
what we're doing in there.
@jyn514
Copy link
Contributor

jyn514 commented Jan 4, 2022

These were added in ec45baa. @inikulin I think I probably lean towards removing them for now - do you have a strong opinion? We still have tests on the host platform, and it seems pretty difficult to try and get this working when cross-compiling ...

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.

None yet

2 participants