mirror of
https://chromium.googlesource.com/crosvm/crosvm
synced 2024-11-24 12:34:31 +00:00
a5f4ea09ad
The timeout for all pool shutdowns was set to 100 ms in a previous change in order to make the tests run faster: <https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4004299> However, this can lead to flakiness if the system is under load while the tests are executing. This change makes some more careful timeout choices to improve reliability without extending the overall time it takes to run the tests. Most of the shutdown_with_timeout(100ms) calls are replaced with the normal shutdown() function, which uses a 5-second timeout. These shutdown() calls are expected to return immediately (either because of a non-timeout error condition or because all pool tasks are already finished), and we can be sure that this 5-second timeout is never relevant because all of the cros_async tests execute in under 5 seconds. This should fix a flaky test that was observed in practice where the disarm_with_pending_work() test would not finish shutting down in the (rather short) 100 ms timeout previously allowed. The shutdown_with_blocked_work timeout is reduced from the original 100 ms to 1 ms, since it should not affect the correctness of the test and will help it run even faster. This test is also renamed to match the actual behavior - it used to mention "panic", but the shutdown function simply returns an error and does not panic. Finally, the shutdown_in_progress() test now uses two different timeouts. This test spawns a thread that waits until shutdown is in progress, then calls the normal shutdown() function to ensure it returns the correct error; this shutdown() call should return immediately, so again the 5-second timeout is not relevant. The main thread of the test now uses a timeout of 200 ms, which should be sufficient to ensure the other thread has time to start up and reach the shutdown() call while observing the shutdown_in_progress flag. This extends the test time somewhat relative to the previous 100 ms timeout, but that is mostly traded off with the timeout reduction in the blocked work test, so the overall test time should be about the same as before. This is probably the most likely test to be flaky going forward, since it relies on OS thread scheduling behavior; if any problems are observed, we can consider making the timeout longer, although this will slow down the tests. BUG=b:306726068 TEST=tools/dev_container tools/presubmit TEST=time cargo test -p cros_async Change-Id: Ic6ec117d616ae7c99a8e3c5a4fe6dd1169c72286 Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4961353 Commit-Queue: Dennis Kempin <denniskempin@google.com> Reviewed-by: Dennis Kempin <denniskempin@google.com> |
||
---|---|---|
.. | ||
src | ||
tests | ||
Cargo.toml |