Refactor flaky tests: Replace sleep_for with synchronization (Issue #188)#189
Merged
Conversation
Replaced blind sleep_for calls with a custom WaitForCondition helper that polls for the expected condition. This eliminates flakiness caused by timing issues and speeds up tests on faster machines. Also added improved_event_bus_test.cpp to the common_system_tests executable in CMakeLists.txt to ensure it is actually run.
Removed sleep_for(100ms) calls that were placed after joining threads. Since thread::join() guarantees that the threads have finished execution, waiting afterwards is unnecessary and only slows down the test suite.
Renamed the helper function in improved_event_bus_test.cpp to match the project's snake_case convention for function names.
kcenon
added a commit
that referenced
this pull request
Apr 13, 2026
) (#189) * Refactor flaky tests in improved_event_bus_test.cpp Replaced blind sleep_for calls with a custom WaitForCondition helper that polls for the expected condition. This eliminates flakiness caused by timing issues and speeds up tests on faster machines. Also added improved_event_bus_test.cpp to the common_system_tests executable in CMakeLists.txt to ensure it is actually run. * Remove redundant sleep_for calls in thread_safety_tests.cpp Removed sleep_for(100ms) calls that were placed after joining threads. Since thread::join() guarantees that the threads have finished execution, waiting afterwards is unnecessary and only slows down the test suite. * Style fix: Rename WaitForCondition to wait_for_condition Renamed the helper function in improved_event_bus_test.cpp to match the project's snake_case convention for function names.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
Refactored unit tests in improved_event_bus_test.cpp and thread_safety_tests.cpp to eliminate flaky behavior caused by hardcoded std::this_thread::sleep_for delays. Introduced a wait_for_condition helper for deterministic
synchronization. Additionally, enabled improved_event_bus_test.cpp in the build system as it was previously excluded.
Type of Change
Related Issues
Fixes #188
Checklist
Testing
Screenshots (if applicable)
N/A
Additional Notes
The improved_event_bus_test.cpp file was previously missing from tests/CMakeLists.txt, meaning these tests were not running in CI. This PR ensures they are built and executed. The wait_for_condition helper was implemented using
snake_case to strictly adhere to the project's coding conventions.