Skip to content

Conversation

dor-forer
Copy link
Collaborator

@dor-forer dor-forer commented Oct 15, 2025

Describe the changes in the pull request

  1. Added new TimeoutGuard class that receives a timeout on creation, and starts a thread that waits on a condition variable until the test is finished (changes the flag on destruction) or the timeout is reached.
  2. Created a new main function that uses Google tests' TestEventListeners mechanism to start TimeoutTestListener (that uses TimeoutGuard), for every test.
  3. Creates SetUp and TearDown functions (Which are inherited from benchmark::Fixture) in BM_VecSimGeneral to create and destroy the timeout when a benchmark is finished.
  4. Disable the TimeoutGuard for EXPECT_EXIT test because they don't work well with the background threading, and added the TimeoutGuard manually to the test.

Which issues this PR fixes

  1. MOD-7846

Main objects this PR modified

  1. unit tests
  2. benchmarks

Mark if applicable

  • This PR introduces API changes
  • This PR introduces serialization changes

Copy link

jit-ci bot commented Oct 15, 2025

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.

@dor-forer dor-forer requested a review from Copilot October 15, 2025 06:42
Copy link
Contributor

@Copilot Copilot AI left a 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 TimeoutGuard class for general timeout protection with configurable actions
  • Created TimeoutTestListener for Google Test integration with automatic per-test timeout
  • Integrated timeout guards into benchmark fixtures using SetUp/TearDown methods

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.

Copy link

codecov bot commented Oct 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.45%. Comparing base (00bf35d) to head (cf512e7).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dor-forer dor-forer requested a review from Copilot October 15, 2025 11:17
Copy link
Contributor

@Copilot Copilot AI left a 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.

@dor-forer dor-forer requested a review from Copilot October 15, 2025 11:29
Copy link
Contributor

@Copilot Copilot AI left a 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.

@dor-forer dor-forer requested a review from Copilot October 15, 2025 12:35
Copy link
Contributor

@Copilot Copilot AI left a 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).
Copy link

Copilot AI Oct 15, 2025

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.

Suggested change
* 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).

Copilot uses AI. Check for mistakes.

@dor-forer dor-forer requested review from alonre24 and meiravgri and removed request for meiravgri October 16, 2025 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant