CI: Add Docker-based compatibility tests, add tests to make quickcheck
#562
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hanno-becker
changed the title
hanno-becker
force-pushed
the
compatibility_tests
branch
3 times, most recently
from
450171a to
0f00d5e
Compare
hanno-becker
changed the title
hanno-becker
force-pushed
the
compatibility_tests
branch
15 times, most recently
from
a5d40bf to
2ae4cbb
Compare
hanno-becker
force-pushed
the
compatibility_tests
branch
6 times, most recently
from
2673d61 to
b49ea96
Compare
hanno-becker
force-pushed
the
compatibility_tests
branch
from
77980fc to
8e1e2a6
Compare
hanno-becker
changed the title
make quickcheck
mkannwischer
force-pushed
the
compatibility_tests
branch
from
8e1e2a6 to
08cba24
Compare
mkannwischer
approved these changes
Dec 22, 2024
There was a problem hiding this comment.
Thanks @hanno-becker - very nice step towards better compatibility. We should really eliminate the tests script in the near future.
I don't think it's a good idea to remove -O3 from the default flags as someone may benchmark mlkem-native with default flags and draw wrong conclusions about performance. I instead changed it to explicitlity setting -O0 for some of the CI jobs.
You may want to add that to more jobs.
Rest looks very good to me if the goal is to eliminatethe test script soon, but then we should really finish it soon.
mkannwischer
added
benchmark
mkannwischer
force-pushed
the
compatibility_tests
branch
from
08cba24 to
cadd396
Compare
Closed
mkannwischer
added
benchmark
mkannwischer
force-pushed
the
compatibility_tests
branch
9 times, most recently
from
8d9d43e to
27526c5
Compare
O3 slows down CI runs, and is not needed except when we benchmark for performance. This commit explicitly sets -O0 for the compatibility and compiler tests to speed-up CI. This also adds an additional variable to pass to the Makefile called EXTRAFLAGS which can be used to overwrite default flags. Those will be appended to the CFLAGS after the default flags. Otherwise, the default optimization flags overwrite the custom flags. Signed-off-by: Hanno Becker <[email protected]> Signed-off-by: Matthias J. Kannwischer <[email protected]>
Signed-off-by: Hanno Becker <[email protected]>
The test script processes the output of test runs with two functions: An `actual_proc` and an `expect_proc`. The `actual_proc` is mostly preprocessing, while the `expect_proc` checks against an expected value. The does not seem to be a reason for this split. This commit conatenates `actual_proc` and `expect_proc` into a single output-checking function `check_proc`. Signed-off-by: Hanno Becker <[email protected]>
This commit adds a basic script META.sh to query the META.json. If jq is present, it's used for the query. Otherwise, some basic fiddling with cat, grep, cut, and tr is used. This is used to conduct a bash-based KAT test in `make quickcheck`, rather then running the test scripts, which already require a Python setup. Signed-off-by: Hanno Becker <[email protected]>
This enables double-checking in the CI logs whether the desired tests were run. Signed-off-by: Hanno Becker <[email protected]>
This will run slightly slower, but hopefully more stable as it is less likely exceeds the maximum request rate for EC2. Signed-off-by: Hanno Becker <[email protected]>
The ACVP client was originally introduced as a standalone script but later added to `tests`, imposing the test infrastructure's dependencies. This commit re-introduces the original standalone script and adds it to `make quickcheck`. The ACVP test within the test scripts remains for now, but should likely be reconciled. Signed-off-by: Hanno Becker <[email protected]>
CPPFLAGS was misnamed. This commit uses EXTRAFLAGS instead. Signed-off-by: Matthias J. Kannwischer <[email protected]>
mkannwischer
force-pushed
the
compatibility_tests
branch
from
27526c5 to
0a769d9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds Docker-based compatibility tests for various Ubuntu-20, Ubuntu-22, Debian, and Amazon Linux docker images; so far, AArch64 only.
A number of changes had to be made, esp. to the test scripts; see commits, paving the way for the removal of
tests.