-
Notifications
You must be signed in to change notification settings - Fork 20
Build all tests for better cross-compilation #126
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
Conversation
dd6eddc to
4842b8b
Compare
|
Hmm, it seems this has caught something already. The macOS x86 CI machines don't support AVX2. Should I:
|
|
I think the only reasonable options within the bounds that libtest gives us is to pass the test. I think we should have one test, which is ignored on CI (i.e. with an environment variable), which requires that you have all the supported levels on your machine. Ideally, we'd point to Qemu to fix this, although I'm not sure how realistic that is. |
6fcbcc4 to
afe33d1
Compare
|
I've updated the tests to operate as per your suggestion: they pass if the SIMD level they're testing is not supported, and there's now one additional test (skipped on CI) that fails if all SIMD levels are not supported. |
DJMcNab
left a comment
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.
Ideally, we'd validate automatically that CI does run all of these tests, but that's begging for flaky CI.
I wonder if it's worth setting up QEMU emulation for this anyway; but that should be a follow-up.
0f1655c to
407420a
Compare
DJMcNab
left a comment
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.
Thanks!
This was discussed a bit in #122. We were previously omitting architecture-specific tests based on the host architecture, which is incorrect and prevents cross-compiling tests to run under something like qemu.
We can't access
CARGO_CFG_TARGET_ARCHat the time that proc macros are evaluated, so we just have to build every architecture-specific test and use#[cfg]to disable ones that aren't applicable to the current architecture.Even more annoyingly, there's no way to conditionally ignore a test at runtime. This means that if the CPU we're running the tests on doesn't support a target feature, we have to either pass or fail that test. I've chosen to fail, to prevent faulty code from appearing to pass tests.
I've also updated the architecture-specific
excludefunctions to add#[ignore]attributes rather than omitting the tests entirely. This makes it more clear that the tests exist, but are not being run because they don't yet pass.