Generate certificates in unit-test fixtures to fix parallel test execution#10767
Generate certificates in unit-test fixtures to fix parallel test execution#10767julianbrost merged 5 commits intomasterfrom
Conversation
16d03ed to
ad696eb
Compare
ad696eb to
1e86025
Compare
yhabteab
left a comment
There was a problem hiding this comment.
Too much low-level boost test framework stuff, but seems to just work fine. I've left just some nitpicking comments, but overall I don't see any major issues. However, why aren't the GHA tests running in parallel now? Instead of postponing that change into the future, I think we should just do it now, as it could help us catch some issues not triggered on my/your local machine.
Agreed. I'll add a commit to change the GHAs and ContainerImage to use the parallel tests. |
1e86025 to
529d42e
Compare
I agree, but it's supported low-level functionality that hasn't changed in ages, so the added maintenance burden should be minimal. And registering test-cases programmatically is something every modern C++ unit-test framework has, so migrating to a different one (like doctest or catch2 for example) should be as easy as switching out the function calls to register the new test cases. Many even have an identical syntax to add decorators. |
529d42e to
a8a32d5
Compare
c5633ba to
383dc5f
Compare
Mostly unrelated change to add the network label to test suites already touched by other changes in this PR. The label is already used for the perfdata writer tests to make it easy to exclude tests that need a network connection when building and testing in a sandbox.
This is necessary for parallel test execution so that tests don't overwrite or delete (destructor of the fixture) data in these directories. This is implemented by generating a unique name for each test with `Utility::NewUniqueId()`.
01d5a2b to
1e4f1ec
Compare
e66682f to
a05e2a3
Compare
|
I've given up on enabling and fixing the parallel builds for the Container Image for now and will do that in a separate PR so we can review and possibly merge this in the meantime. |
|
Shouldn't this PR also fix #10722? |
Description
Currently in master, parallel test execution is broken, because the tests that need TLS certificates overwrite each other's certificate files in the middle of tests. This PR fixes parallel test execution by generating CTest fixture units to set up each certificate once for all tests that require it and using CTest's dependency resolution to run those units before the actual tests.
Implementation
A new unit-test decorator
RequiresCertificateis added that can be constructed with a list of certificates that this test requires and a prefix domain the test belongs to. Tests from different prefix domains are not run in parallel with each other and a cleanup will be performed in between the groups.On instantiation, this decorator will add CTest fixtures for the setup and cleanup of the CA and required certificates (if no other test has added them yet). CTest fixtures are themselves test-units that are put into a dependency relationship with their associated test-cases by CTest and it is ensured that the setup routine runs before all tests that require the fixture and the cleanup routine runs after these tests. The fixtures are shared by all tests that require the same certificates with the same prefix.
The old
prepare_directory_*andcleanup_certstests that made up thessl_certsfixture were removed in favor of these new fixtures. And theCertificateFixtureBoost.Test fixture (which gets run once per test) no longer does any setup and cleanup on its own. All it does now is copy the certificates from the persistent directory to the local test directory, which has been prefixed with a UUID to make it unique between tests.Rationale
The generation of the fixture units is fairly complex for currently not a lot of (current) usage, but it is a general proper solution, unlike the increasingly ugly workarounds that the lazy generation of certificates and manually added
ssl_certs_*fixtures that do much other than enforce ordering between the tests. Recently I attempted to fix another problem this caused in #10749, which added more workarounds, but I should have gone with this solution instead.Parallel execution of tests is the default on for example the build scripts of Gentoo and maybe other distros and our CI could also benefit from it. See below for a comparison of serialized vs parallel test execution. Just requiring downstream to turn off parallel testing because our tests break with it would also not be a good look so this fixes it to run as much in parallel as possible.
Before
After
Other Changes
Since I was editing those lines anyway I also added the
networklabel to the http unit-tests which fixes #10722.