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.

Sign up for GitHub

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

Jump to bottom

Rationalize Valgrind tests #6685

Merged
merged 5 commits into from
Dec 6, 2022
Merged

Rationalize Valgrind tests #6685

merged 5 commits into from
Dec 6, 2022

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Nov 29, 2022 •
edited
Loading

  • Fix test gap: Test MBEDTLS_USE_PSA_CRYPTO with Valgrind as well. We were only testing the default config.
  • CI performance: When testing with Valgrind for constant flow, don't repeat the testing of suites without constant flow annotations: that would not do anything extra compared to the basic Valgrind components.
  • Maintenance: Move constant-flow tests from test_suite_ssl into their own test suites.

Supersedes #6665.

Gatekeeper checklist

  • changelog N/A (tests only)
  • backport 2.28
  • tests N/A

@gilles-peskine-arm
Move constant-time HMAC testing to its own suite
77e68c3 
These are very CPU-intensive, so make it easy to skip them. And conversely,
make it easy to run them without the growing body of SSL tests.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm
Create a separate test suite for constant-time functions
3ffd6bc 
This is the first step in arranging that functions from constant_time.c are
tested in test_suite_constant_time.function.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm
Test MBEDTLS_USE_PSA_CRYPTO with Valgrind
df3dd4c 
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm
Valgrind for constant flow: skip non-CF test suites
0c67160 
When testing under Valgrind for constant flow, skip test suites that don't
have any constant-flow annotations, since the testing wouldn't do anything
more that testing with ordinary Valgrind (component_test_valgrind and
component_test_valgrind_psa). This is a significant time saving since
testing with Valgrind is very slow.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, needs-backports Backports are missing or are pending review and approval. needs-ci Needs to pass CI tests size-s Estimated task size: small (~2d) component-test Test framework and CI scripts labels Nov 29, 2022
@gilles-peskine-arm gilles-peskine-arm mentioned this pull request Nov 29, 2022
Closed
3 tasks
daverodgman
daverodgman previously approved these changes Nov 29, 2022
View reviewed changes
@gilles-peskine-arm
Fix the build with MBEDTLS_TEST_HOOKS && !MBEDTLS_USE_PSA_CRYPTO
d1bc6da 
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm gilles-peskine-arm removed the needs-ci Needs to pass CI tests label Nov 30, 2022
@gilles-peskine-arm gilles-peskine-arm added the priority-medium Medium priority - this can be reviewed as time permits label Nov 30, 2022
davidhorstmann-arm
davidhorstmann-arm approved these changes Dec 1, 2022
View reviewed changes
Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@davidhorstmann-arm davidhorstmann-arm removed the needs-review Every commit must be reviewed by at least two team members, label Dec 1, 2022
@gilles-peskine-arm gilles-peskine-arm mentioned this pull request Dec 1, 2022
Merged
3 tasks
@gilles-peskine-arm gilles-peskine-arm added the approved Design and code approved - may be waiting for CI or backports label Dec 1, 2022
@daverodgman daverodgman merged commit 1fe4529 into Mbed-TLS:development Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daverodgman daverodgman daverodgman approved these changes

@davidhorstmann-arm davidhorstmann-arm davidhorstmann-arm approved these changes

Assignees
No one assigned
Labels
approved Design and code approved - may be waiting for CI or backports component-test Test framework and CI scripts needs-backports Backports are missing or are pending review and approval. priority-medium Medium priority - this can be reviewed as time permits size-s Estimated task size: small (~2d)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gilles-peskine-arm @daverodgman @davidhorstmann-arm