Skip to content

fix: AdaptiveTimeout MinTimeout test needs to be more resource-aware for github runners - BED-7153#281

Merged
definitelynotagoblin merged 1 commit intov4from
anemeth/mintimeout-test-fix
Mar 13, 2026
Merged

fix: AdaptiveTimeout MinTimeout test needs to be more resource-aware for github runners - BED-7153#281
definitelynotagoblin merged 1 commit intov4from
anemeth/mintimeout-test-fix

Conversation

@definitelynotagoblin
Copy link
Contributor

@definitelynotagoblin definitelynotagoblin commented Mar 13, 2026

Description

Github runners are too efficient with concurrency resources for Task.Delay() code in parallelized tests to be particularly timely.

Motivation and Context

https://specterops.atlassian.net/browse/BED-7153

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Chore (a change that does not modify the application functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Documentation updates are needed, and have been made accordingly.
  • I have added and/or updated tests to cover my changes.
  • All new and existing tests passed.
  • My changes include a database migration.

Summary by CodeRabbit

  • Tests
    • Updated adaptive timeout test logic to improve test reliability and clarity.

@definitelynotagoblin definitelynotagoblin self-assigned this Mar 13, 2026
@definitelynotagoblin definitelynotagoblin added the bug Something isn't working label Mar 13, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8ddee92e-1194-4dd5-acba-76e7b0ed5544

📥 Commits

Reviewing files that changed from the base of the PR and between c6fb618 and b65a3b3.

📒 Files selected for processing (1)
  • test/unit/AdaptiveTimeoutTest.cs

Walkthrough

A test in the AdaptiveTimeout suite was modified to remove a 50ms delay and simplify its assertions. The test now uses Task.CompletedTask for immediate execution and removes the secondary assertion validating timeout boundaries.

Changes

Cohort / File(s) Summary
Test Simplification
test/unit/AdaptiveTimeoutTest.cs
Modified AdaptiveTimeout_GetAdaptiveTimeout_MinTimeout test to remove timeout wait (50ms delay replaced with Task.CompletedTask) and eliminated assertion checking adaptive timeout is less than maxTimeout; retained only minTimeout equality check.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A test once waited, now it races fast,
Delays removed—immediacy at last!
Assertions lean, the path runs clean,
Swift as a rabbit hopping between,
Where timeout dwelt, now speed is blessed.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description addresses motivation/context and references the linked issue (BED-7153), but the 'How Has This Been Tested?' section is incomplete with only placeholder comments remaining. Complete the 'How Has This Been Tested?' section with specific details about how the test fix was validated and what testing environment was used.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: fixing an AdaptiveTimeout MinTimeout test to be more resource-aware for GitHub runners, with issue reference.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch anemeth/mintimeout-test-fix
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@definitelynotagoblin definitelynotagoblin merged commit 15abb15 into v4 Mar 13, 2026
4 checks passed
@definitelynotagoblin definitelynotagoblin deleted the anemeth/mintimeout-test-fix branch March 13, 2026 18:32
@github-actions github-actions bot locked and limited conversation to collaborators Mar 13, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants