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

CMake: generate the list of test suites automatically #5463

Merged
merged 7 commits into from
Feb 7, 2022
Merged

CMake: generate the list of test suites automatically #5463

merged 7 commits into from
Feb 7, 2022

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Jan 25, 2022 •
edited
Loading

We keep forgetting to register new test suites in tests/CMakeLists.txt. To
fix this problem once and for all, remove the need for manual registration.

The following test suites were missing:

test_suite_cipher.aria
test_suite_psa_crypto_driver_wrappers
test_suite_psa_crypto_generate_key.generated

Backport: #5465

@gilles-peskine-arm
CMake: generate the list of test suites automatically
29080e8 
We keep forgetting to register new test suites in tests/CMakeLists.txt. To
fix this problem once and for all, remove the need for manual registration.

The following test suites were missing:
  test_suite_cipher.aria
  test_suite_psa_crypto_driver_wrappers
  test_suite_psa_crypto_generate_key.generated

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm gilles-peskine-arm added bug enhancement 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 needs-reviewer This PR needs someone to pick it up for review labels Jan 25, 2022
@gilles-peskine-arm gilles-peskine-arm added the size-s Estimated task size: small (~2d) label Jan 25, 2022
@gilles-peskine-arm gilles-peskine-arm mentioned this pull request Jan 25, 2022
Draft
@gilles-peskine-arm gilles-peskine-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Jan 25, 2022
@gilles-peskine-arm
Remove accidental use of a feature that doesn't exist in CMake 3.5
cd55fe0 
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm
Fix string/regexp backslash escapes
a18fb9d 
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm gilles-peskine-arm added component-platform Portability layer and build scripts and removed needs-work labels Jan 25, 2022
@gilles-peskine-arm gilles-peskine-arm mentioned this pull request Jan 25, 2022
Merged
@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review and removed needs-ci Needs to pass CI tests labels Jan 26, 2022
AndrzejKurek
AndrzejKurek previously approved these changes Jan 27, 2022
View reviewed changes
d3zd3z
d3zd3z previously approved these changes Feb 2, 2022
View reviewed changes
@mpg mpg added approved Design and code approved - may be waiting for CI or backports needs-ci Needs to pass CI tests and removed needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Feb 3, 2022
@mpg
Copy link
Contributor

mpg commented Feb 3, 2022

@gilles-peskine-arm The CI fails in test_cmake_as_subdirectory (only in the pr-merge job, though).

@gilles-peskine-arm
Copy link
Contributor Author

@mpg Thanks for letting me know! Previous pr-merge jobs passed, so this is a conflict with #5385. I can reproduce the failure on my machine, but I don't understand it yet. Once I find a fix I'll probably push a new version of this PR rebased on top of the merge of #5385.

@gilles-peskine-arm gilles-peskine-arm added needs-work and removed approved Design and code approved - may be waiting for CI or backports needs-backports Backports are missing or are pending review and approval. labels Feb 3, 2022
@gilles-peskine-arm
Copy link
Contributor Author

Actually the failure is non-deterministic. #5385 is probably a red herring.

@gilles-peskine-arm
Copy link
Contributor Author

I can reproduce the failure of test_cmake_as_subdirectory intermittently on Ubuntu 16.04. I can't reproduce it on Ubuntu 20.04. I think this is in fact the known issue #5223, which we decided to work around rather than fix. I guess we should expand the workaround to other components that do an out-of-source build: test_cmake_as_subdirectory, test_cmake_as_package, test_cmake_as_package_install.

@gilles-peskine-arm
Stop CMake out of source tests running on 16.04 (continued)
a300099 
The race condition mentioned in the previous commit
"Stop CMake out of source tests running on 16.04"
has also been observed with test_cmake_as_subdirectory and can presumably
happen with test_cmake_as_package and test_cmake_as_package_install as well.
So skip all of these components on Ubuntu 16.04.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm
Remove obsolete cd at the end of a component
3bc3409 
This is no longer useful now that components run in a subshell.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm
Remove obsolete variable restoration or unset at the end of a component
c1247c0 
This is no longer useful now that components run in a subshell.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm
Remove obsolete calls to if_build_succeeded
827dbd9 
This is now a no-op.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm
Copy link
Contributor Author

I took the same workaround for #5223 from #5233 and applied it to the similar cmake jobs. I also removed a bit of dead code I noticed in all.sh. I made similar patches to the backport.

@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review and removed needs-ci Needs to pass CI tests needs-reviewer This PR needs someone to pick it up for review labels Feb 4, 2022
@mpg mpg added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels Feb 7, 2022
@mpg mpg merged commit d81e774 into Mbed-TLS:development Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndrzejKurek AndrzejKurek AndrzejKurek approved these changes

@d3zd3z d3zd3z d3zd3z approved these changes

Assignees
No one assigned
Labels
approved Design and code approved - may be waiting for CI or backports bug component-platform Portability layer and build scripts enhancement size-s Estimated task size: small (~2d)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gilles-peskine-arm @mpg @d3zd3z @AndrzejKurek