-
Notifications
You must be signed in to change notification settings - Fork 22
Add test and benchmarks Timeout Guards [MOD-7846] #800
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset. In case there are security findings, they will be communicated to you as a comment inside the PR. Hope you’ll enjoy using Jit. Questions? Comments? Want to learn more? Get in touch with us. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces timeout protection mechanisms for both unit tests and benchmarks to prevent indefinite hangs and improve CI reliability. The implementation uses RAII-style timeout guards that monitor test/benchmark execution and forcefully terminate if timeouts are reached.
Key changes:
- Added
TimeoutGuardclass for general timeout protection with configurable actions - Created
TimeoutTestListenerfor Google Test integration with automatic per-test timeout - Integrated timeout guards into benchmark fixtures using
SetUp/TearDownmethods
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/utils/timeout_guard.h | Core timeout implementation with RAII guards and benchmark wrapper |
| tests/utils/timeout_test_environment.h | Google Test listener for automatic test timeout registration |
| tests/utils/test_main_with_timeout.cpp | Custom main function registering global timeout protection |
| tests/unit/test_common.cpp | Removed manual timeout code, added timeout_guard.h include |
| tests/unit/CMakeLists.txt | Updated test executables to use custom main and link against gtest |
| tests/benchmark/bm_vecsim_general.h | Added timeout guard to benchmark fixture with SetUp/TearDown |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…ailure if any test failed again
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #800 +/- ##
==========================================
- Coverage 96.86% 96.45% -0.41%
==========================================
Files 126 126
Lines 7739 7739
==========================================
- Hits 7496 7465 -31
- Misses 243 274 +31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…to dorer-test-timeout-guard_MOD-7846
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| * @brief Custom main function that registers global timeout protection for all tests. | ||
| * | ||
| * This replaces gtest_main and adds automatic timeout protection to every test. | ||
| * Tests will automatically timeout after 30 seconds (or customized duration based on test type). |
Copilot
AI
Oct 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation inaccuracy: The comment states tests timeout after 30 seconds, but the code sets a 100-second timeout on line 23. Update the comment to reflect the actual timeout value.
| * Tests will automatically timeout after 30 seconds (or customized duration based on test type). | |
| * Tests will automatically timeout after 100 seconds (or customized duration based on test type). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice job!
| // Get timeout for this specific test (can be customized per test) | ||
| auto timeout = GetTimeoutForTest(test_info); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add one test for testing this mechanism as well, using a lower timeout?
Describe the changes in the pull request
TimeoutGuardclass that receives atimeouton creation, and starts a thread that waits on a condition variable until the test is finished (changes the flag on destruction) or thetimeoutis reached.TestEventListenersmechanism to startTimeoutTestListener(that usesTimeoutGuard), for every test.SetUpandTearDownfunctions (Which are inherited from benchmark::Fixture) inBM_VecSimGeneralto create and destroy the timeout when a benchmark is finished.TimeoutGuardfor EXPECT_EXIT test because they don't work well with the background threading, and added theTimeoutGuardmanually to the test.Which issues this PR fixes
Main objects this PR modified
Mark if applicable