-
Notifications
You must be signed in to change notification settings - Fork 81
ci: Run tests also on i686 #283
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
80b6a97
to
28ba599
Compare
3712940
to
0c1a6e2
Compare
1a864d3
to
f7646c9
Compare
#282 has already been fixed... Should this be closed or is there anything valuable that we can scavenge? 😁 |
This is about running the tests on i386, not just building as with the ulong size on 32b arch, some things might explode even during runtime. I tried to rework it, but did not manage to finalize the linking on multiarch yet. |
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
0d8d37c
to
a8ccd1f
Compare
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Okay, I think this looks good and should be merged 🚀 One question though, couldn't we adjust the test to use I'll approve the thing anyway 😅 |
Good idea! Its always better to fix the test than skip it. I will adjust it. I was probably too focused on fixing the CI, rather than fixing the tests. |
Actually the value But actually this already fails non-gracefully in the test itself if changed to this value, as it looks like the max vector capacity is different on the 32b arch rust:
So yes, I can probably try to find the vector capacity, but this would be already different test. This test tries this input specially made for the specs (commented in the code) rust-cryptoki/cryptoki/src/mechanism/aead.rs Lines 47 to 49 in 83790f2
So reverting back my first comment. Its probably best to skip it on 32b arch in the end. Good to get merged. |
Oh yes, definitely. Sorry to send you on such a rabbit chase I thought it will be "just a simple fix" 😉 Merging, thanks for your work! 🙇 |
Fixes: #285