Skip to content

Fix: Simplify TestHttpServer Drop logic to prevent test flakiness #2495

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

dlachaume
Copy link
Collaborator

@dlachaume dlachaume commented May 19, 2025

Content

This PR includes an update to the Drop implementation of TestHttpServer, removing the recv_timeout call that was previously used to wait for the server thread to signal its shutdown.

This change fixes flakiness in CI tests, likely caused by the blocking behavior of block_on, which can delay the server thread's shutdown signal.

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • CHANGELOG file is updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • All check jobs of the CI have succeeded
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested
  • Documentation
    • Update README file (if relevant)
    • Update documentation website (if relevant)
    • Add dev blog post (if relevant)
    • Add ADR blog post or Dev ADR entry (if relevant)
    • No new TODOs introduced

@dlachaume dlachaume self-assigned this May 19, 2025
Copy link

github-actions bot commented May 19, 2025

Test Results

    3 files  ±0     77 suites  ±0   14m 12s ⏱️ - 1m 9s
1 913 tests ±0  1 912 ✅  - 1  0 💤 ±0  1 ❌ +1 
3 225 runs  ±0  3 224 ✅  - 1  0 💤 ±0  1 ❌ +1 

For more details on these failures, see this check.

Results for commit eb9b803. ± Comparison against base commit 2b88e17.

♻️ This comment has been updated with latest results.

@dlachaume dlachaume temporarily deployed to testing-preview May 19, 2025 12:52 — with GitHub Actions Inactive
@dlachaume dlachaume force-pushed the dlachaume/client-integration-test-flaky branch from 09fc141 to cff69a8 Compare May 19, 2025 13:56
@dlachaume dlachaume marked this pull request as draft May 19, 2025 14:04
@dlachaume dlachaume force-pushed the dlachaume/client-integration-test-flaky branch 2 times, most recently from bf434a8 to 5df4c69 Compare May 19, 2025 14:35
@dlachaume dlachaume force-pushed the dlachaume/client-integration-test-flaky branch from 5df4c69 to f98300d Compare May 19, 2025 15:32
@dlachaume dlachaume changed the title Fix: Increase timeout in TestHttpServer to reduce test flakiness Fix: Simplify TestHttpServer Drop logic to prevent test flakiness May 19, 2025
@dlachaume dlachaume force-pushed the dlachaume/client-integration-test-flaky branch from f98300d to c2db17f Compare May 19, 2025 16:35
@dlachaume dlachaume marked this pull request as ready for review May 19, 2025 16:36
@dlachaume dlachaume temporarily deployed to testing-preview May 19, 2025 16:47 — with GitHub Actions Inactive
@dlachaume dlachaume force-pushed the dlachaume/client-integration-test-flaky branch from c2db17f to eb9b803 Compare May 20, 2025 12:16
Copy link
Collaborator

@turmelclem turmelclem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dlachaume dlachaume force-pushed the dlachaume/client-integration-test-flaky branch from 49f2cf0 to 46d2967 Compare May 20, 2025 12:43
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.

2 participants