Skip to content

Conversation

@asmacdo
Copy link
Member

@asmacdo asmacdo commented Jan 6, 2026

Hopefully Fixes: #190

Signal tests fail sometimes, probably due to a race condition. On slow startup on some CI systems (I've never seen locally) the signal could be sent before the signal handler is registered. If that is whats happening, the test fails, and on the next rerun the wait time inceases.

With this retry logic, it seems worth also dropping the wait time a little bit.

@CodyCBakerPhD this will hopefully solve a lot of the flake you've been seeing.
@candleindark tagging you too since you recently dug into signal handling :)

Signal tests fail sometimes, probably due to a race condition.
On slow startup on some CI systems (I've never seen locally) the signal
could be sent before the signal handler is registered. If that is whats
happening, the test fails, and on the next rerun the wait time inceases.
Copilot AI review requested due to automatic review settings January 6, 2026 22:47
@asmacdo asmacdo added the semver-tests Add or improve existing tests label Jan 6, 2026
@codecov
Copy link

codecov bot commented Jan 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.98%. Comparing base (44e25ba) to head (8c26f52).
⚠️ Report is 28 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #359   +/-   ##
=======================================
  Coverage   90.98%   90.98%           
=======================================
  Files           8        8           
  Lines        1021     1021           
  Branches      135      135           
=======================================
  Hits          929      929           
  Misses         70       70           
  Partials       22       22           

☔ 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.

Copy link
Contributor

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 aims to address flaky signal handling tests that occasionally fail on slow CI systems due to race conditions where signals are sent before handlers are registered. The changes add retry logic using pytest-rerunfailures and attempt to scale wait times based on retry attempts.

  • Adds @pytest.mark.flaky(reruns=5) decorator to both signal tests to enable automatic retries
  • Injects the request fixture to access attempt/execution count information
  • Implements dynamic wait time scaling based on retry attempts (0.2 * attempt)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@asmacdo asmacdo marked this pull request as draft January 6, 2026 22:57
@asmacdo asmacdo requested a review from Copilot January 6, 2026 23:05
Copy link
Contributor

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 1 out of 1 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@CodyCBakerPhD
Copy link
Contributor

Who knows, might not be necessary after all

Recent runs on #351 have looked pretty stable

@asmacdo
Copy link
Member Author

asmacdo commented Jan 6, 2026

I'll leave this open as a draft then-- race condition flakes have haunted the project for a while, so this pattern might prove to be useful elsewhere.

@asmacdo asmacdo requested a review from candleindark January 8, 2026 19:11
@asmacdo
Copy link
Member Author

asmacdo commented Jan 8, 2026

@candleindark this will hopefully solve the flakey tests you saw.

@asmacdo asmacdo requested a review from CodyCBakerPhD January 9, 2026 16:51
@asmacdo
Copy link
Member Author

asmacdo commented Jan 9, 2026

@CodyCBakerPhD maybe reviewing this so I can merge before more intel work would save you a headache :)

@CodyCBakerPhD
Copy link
Contributor

Can't hurt?

@asmacdo asmacdo marked this pull request as ready for review January 9, 2026 16:52
@asmacdo asmacdo merged commit b7dc6b8 into con:main Jan 9, 2026
28 of 29 checks passed
@asmacdo asmacdo mentioned this pull request Jan 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-tests Add or improve existing tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI Test Flake

2 participants