Remove wall-clock sleeps from exponential backoff unit test

[coderabbitai]

Problem

The test test_accept_exponential_backoff_doubles_and_caps() in src/server/config/tests.rs (lines 249-295) uses wall-clock sleeps which introduce flakiness and slow down the test suite.

Solution

Replace the wall-clock sleep approach with deterministic assertion of the backoff schedule by extracting a pure function that computes the backoff sequence.

Example Refactor

/// Behaviour test verifying exponential delay doubling and capping (pure logic).
#[test]
fn test_accept_exponential_backoff_doubles_and_caps() {
    use std::time::Duration;

    let initial = Duration::from_millis(10);
    let max = Duration::from_millis(80);
    let attempts = 5usize;

    let mut backoff = initial;
    let mut sequence = Vec::with_capacity(attempts);
    for _ in 0..attempts {
        sequence.push(backoff);
        backoff = std::cmp::min(backoff.saturating_mul(2), max);
    }

    let expected = [
        initial,
        std::cmp::min(initial * 2, max),
        std::cmp::min(initial * 4, max),
        std::cmp::min(initial * 8, max),
        max,
    ];
    assert_eq!(&sequence[..], &expected);
}

Benefits

• Eliminates test flakiness from timing-dependent assertions
• Significantly faster test execution
• More deterministic and reliable test behaviour
• Validates the core doubling-and-capping logic without timing concerns

Backlinks

• PR: Type preamble handler test cases and stabilise runtime readiness #301
• Comment: Type preamble handler test cases and stabilise runtime readiness #301 (comment)...

[leynos]

@coderabbitai looking at the codebase, is this issue still relevant? If the issue has been fixed in the code, does the documentation reflect this?

If it is still relevant, please provide an assessment of the issue criticality (critical, high, medium or low), based on risk of the issue impacting the user and the anticipated blast radius.\n\nOnce a decision has been made, apply the appropriate label.

If the issue is documentation ONLY (i.e., no code changes needed), apply the documentation label in addition to the appropriate criticality label.

Please also provide an impact of the required changes.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch issue `#310` details
gh issue view 310 --json title,body,labels,state,author,createdAt

Length of output: 1750

---

🏁 Script executed:

#!/bin/bash
# Check if the test file exists and look at the specific test
fd -p "src/server/config/tests.rs" --exec cat {}

Length of output: 13851

---

🏁 Script executed:

#!/bin/bash
# Check for any testing documentation or guidelines
fd -t f -e md . | xargs grep -l -i "test\|backoff" 2>/dev/null | head -10

Length of output: 410

---

🏁 Script executed:

#!/bin/bash
# Check if any documentation mentions exponential backoff or this specific test
rg -i "exponential.*backoff|test_accept_exponential_backoff" docs/ -A 3 -B 3

# Also check for any general testing guidelines
rg -i "wall.*clock|sleep.*test|flaky.*test" docs/ -A 2 -B 2

Length of output: 3561

---

🏁 Script executed:

#!/bin/bash
# Apply the "medium" label to issue `#310`
gh issue edit 310 --add-label "medium"

Length of output: 46

---

@leynos, I've analysed issue #310 and the codebase. Here's my assessment:

Relevance: STILL RELEVANT

The issue has NOT been fixed. The test test_accept_exponential_backoff_doubles_and_caps() at lines 249-284 in src/server/config/tests.rs still uses the problematic wall-clock sleep approach:

• Line 269: thread::sleep(backoff) - actual wall-clock delay
• Lines 270-271: Measures elapsed time with Instant::now()
• Lines 277-283: Timing assertions with 20ms tolerance windows

Documentation Status

The documentation actually supports this issue's resolution. In docs/behavioural-testing-in-rust-with-cucumber.md, the project explicitly warns:

"Avoid Arbitrary Sleeps: Never use fixed delays to wait for an event. The correct duration is impossible to guess and will lead to either slow tests or flaky tests."

The current test violates this documented best practice. However, the documentation doesn't specifically mention this issue or provide guidance on the fix.

Criticality: MEDIUM ⚠️

Risk Assessment:

• User impact risk: None (test-only, no production code affected)
• Blast radius: Very low (single test in CI/test suite)
• Developer impact: Moderate (flaky CI failures, slower test execution, violates best practices)

Justification: While this doesn't affect production users, it creates technical debt that impacts developer productivity through potential flaky test failures and unnecessarily slow test runs (230ms+ of sleeping). It also contradicts the project's documented testing principles.

Impact of Required Changes: LOW

Scope: Single test function refactor (~35 lines)

Effort: Low complexity

• Extract pure backoff sequence calculation
• Replace wall-clock timing with deterministic assertions
• No changes to production code required

Risk: Very low (test-only change, no production impact)

Benefits:

• Eliminates test flakiness completely
• ~230ms faster test execution
• Deterministic, reliable behaviour
• Aligns with documented best practices

Label Applied

✅ Applied medium label to issue #310

[leynos]

@CodeRabbit plan

[coderabbitai]

Coding Plan

Approach

Refactor the test_accept_exponential_backoff_doubles_and_caps() test in src/server/config/tests.rs to eliminate wall-clock sleeps, replacing timing-based validation with deterministic assertions that verify the pure backoff computation logic.

Observations

The project is a Rust-based server implementation with exponential backoff logic for accept loop failures. The backoff computation (doubling delay, capped at max) is currently inline in src/server/runtime.rs. Testing coverage includes an async integration test in src/server/runtime/tests.rs using tokio::time::pause for deterministic time control, and a synchronous unit test in src/server/config/tests.rs that currently uses wall-clock thread::sleep() calls. The codebase has established patterns for deterministic time testing using Tokio's virtual time features.

Implementation Steps

Refactor Backoff Unit Test to Deterministic Assertions

This phase replaces the timing-dependent test with a pure logic test that validates the exponential backoff algorithm without any wall-clock delays.

Task 1: Remove Wall-Clock Sleep Dependencies from Backoff Test

Refactor the test_accept_exponential_backoff_doubles_and_caps() function in src/server/config/tests.rs to use deterministic assertions instead of timing measurements.

• Remove imports of std::thread and timing-related constructs (Instant, wall-clock Duration measurements)
• Remove the sleep loop that performs actual delays between iterations
• Remove tolerance-based timing assertions that compare elapsed time

Task 2: Implement Pure Computation Validation

Replace the timing-based validation with direct computation and assertion of the backoff sequence.

• Define test parameters: initial delay (10ms), max delay (80ms), and number of attempts (5)
• Build the backoff sequence by iterating and applying the doubling-with-cap formula: std::cmp::min(backoff.saturating_mul(2), max)
• Construct the expected sequence array with explicit values showing the progression
• Assert equality between the computed sequence and expected values
• Retain the #[test] attribute (synchronous test, no async runtime needed)

🤖 Prompt for AI agents

Refactor the `test_accept_exponential_backoff_doubles_and_caps()` test in
`src/server/config/tests.rs` to validate exponential backoff logic using
deterministic assertions without wall-clock delays.

**Remove timing-based test dependencies:**
- Remove imports: `std::thread`, `Instant`, and wall-clock `Duration`
measurement constructs
- Remove the sleep loop that performs actual delays between iterations
- Remove tolerance-based timing assertions that compare elapsed time

**Implement pure computation validation:**
- Define test parameters:
  - `initial` delay: 10ms
  - `max` delay: 80ms
  - Number of `attempts`: 5
- Build the backoff sequence by iterating through attempts and applying the
doubling-with-cap formula: `std::cmp::min(backoff.saturating_mul(2), max)`
- Construct the expected sequence array with explicit values showing the
exponential progression with cap
- Assert equality between the computed sequence and expected values
- Keep the `#[test]` attribute (remains a synchronous test)

---

🚀 Next Steps

🤖 All AI agent prompts combined

Refactor the `test_accept_exponential_backoff_doubles_and_caps()` test in
`src/server/config/tests.rs` to validate exponential backoff logic using
deterministic assertions without wall-clock delays.

**Remove timing-based test dependencies:**
- Remove imports: `std::thread`, `Instant`, and wall-clock `Duration`
measurement constructs
- Remove the sleep loop that performs actual delays between iterations
- Remove tolerance-based timing assertions that compare elapsed time

**Implement pure computation validation:**
- Define test parameters:
  - `initial` delay: 10ms
  - `max` delay: 80ms
  - Number of `attempts`: 5
- Build the backoff sequence by iterating through attempts and applying the
doubling-with-cap formula: `std::cmp::min(backoff.saturating_mul(2), max)`
- Construct the expected sequence array with explicit values showing the
exponential progression with cap
- Assert equality between the computed sequence and expected values
- Keep the `#[test]` attribute (remains a synchronous test)

💡 Iterate on the plan with: @coderabbitai <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For assumption 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!