Increase socket retry budget for CI reliability

[coderabbitai]

Description

Raise the retry count to 20 and the delay to 25 ms in test-support so slow runners don't time out during CI execution.

Changes Required

In test-support/src/util.rs

• Change SOCKET_RETRY_COUNT from 10 to 20
• Change SOCKET_RETRY_DELAY from Duration::from_millis(10) to Duration::from_millis(25)

In test-support/src/lib.rs

• Update the commented example usages to reflect the new values

Proposed Implementation

--- a/test-support/src/util.rs
+++ b/test-support/src/util.rs
@@ -7,7 +7,7 @@ pub const SOCKET_RETRY_COUNT: u32 = 10;
 pub const SOCKET_RETRY_COUNT: u32 = 20;
 
- pub const SOCKET_RETRY_DELAY: Duration = Duration::from_millis(10);
+ pub const SOCKET_RETRY_DELAY: Duration = Duration::from_millis(25);

 // Wait for a file to appear, retrying on failure
 pub async fn wait_for_file(…

--- a/test-support/src/lib.rs
+++ b/test-support/src/lib.rs
@@ -20,7 +20,7 @@ pub use util::{SOCKET_RETRY_COUNT, SOCKET_RETRY_DELAY};
 // use test_support::{wait_for_file, SOCKET_RETRY_COUNT, SOCKET_RETRY_DELAY};
 // let found = wait_for_file(path, SOCKET_RETRY_COUNT, SOCKET_RETRY_DELAY).await;
 // SOCKET_RETRY_COUNT = 20, SOCKET_RETRY_DELAY = 25 ms

Context

Related to PR #26: #26
Requested by: @leynos

[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, based on risk of the issue impacting the user and the anticipated blast radius.

Please also provide an impact of the required changes.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's find and examine the test-support files mentioned in the issue
echo "=== Searching for test-support directory structure ==="
fd -t d test-support

echo -e "\n=== Looking for util.rs in test-support ==="
fd util.rs test-support

echo -e "\n=== Looking for lib.rs in test-support ==="
fd lib.rs test-support

Length of output: 459

---

🏁 Script executed:

#!/bin/bash
# Now let's examine the current values in util.rs to see if the issue has been addressed
echo "=== Current content of test-support/src/util.rs ==="
cat test-support/src/util.rs

echo -e "\n=== Current content of test-support/src/lib.rs ==="
cat test-support/src/lib.rs

Length of output: 4723

---

@leynos Looking at the current codebase, the issue is still relevant and has not been addressed yet.

Current State Analysis

The values in test-support/src/util.rs are still at their original settings:

• SOCKET_RETRY_COUNT: u32 = 10 (should be 20)
• SOCKET_RETRY_DELAY: Duration = Duration::from_millis(10) (should be 25ms)

The documentation in test-support/src/lib.rs is consistent with the current values and properly documents the constants.

Criticality Assessment

Risk Level: Medium-Low

• User Impact: No direct impact on end users since this affects only test infrastructure
• Developer Impact: Moderate - flaky CI tests due to timeouts can cause false negatives, slowing down development workflow
• Blast Radius: Limited to CI/test execution environment
• Failure Mode: Intermittent test failures on slower CI runners, potentially blocking legitimate PRs

Impact of Required Changes

Positive Impacts:

• Improved CI reliability on slower runners
• Reduced false negative test failures
• Better developer experience with more stable test suite
• Total maximum wait time increases from 100ms (10×10ms) to 500ms (20×25ms)

Potential Concerns:

• Slightly longer test execution time in worst-case scenarios
• Tests may take longer to fail when there are genuine issues

Risk Assessment:

• Very Low Risk: Changes only affect test constants
• No Breaking Changes: API remains unchanged
• Minimal Performance Impact: Additional delay only occurs during actual failures

Recommendation

Implement the proposed changes. The benefits of improved CI stability significantly outweigh the minimal cost of slightly longer test timeouts. This is a low-risk change that addresses a real infrastructure reliability issue.

      /|   /|  
     ( :v:  ) 
      |(_)|
     /  _  \
    /  | |  \
   (   \-/   )
    \_______/

[leynos]

@coderabbitai 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.

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

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@leynos Based on the impact analysis, I assess this issue as low criticality.

Criticality Rationale

• No End-User Impact: This affects only test infrastructure, not production functionality
• Limited Blast Radius: Confined to CI environment and developer workflow
• Low Risk: Configuration changes with minimal potential for negative consequences
• Non-Blocking: Current system functions, this is a reliability improvement rather than a critical fix

The issue involves code changes (updating constants in util.rs), so this is not documentation-only.

---

🏁 Script executed:

#!/bin/bash
# Apply the "low" criticality label to issue #41
gh issue edit 41 --add-label "low"

Length of output: 42

---

@leynos The issue criticality has been assessed as low and the label has been successfully applied to issue #41.

Summary

Criticality: LOW

• Affects only test infrastructure, not end users
• Limited to CI environment with minimal blast radius
• Low-risk configuration changes to improve reliability
• Non-urgent improvement rather than critical fix

The label has been applied to the issue. This change will improve CI stability without introducing significant risk to the codebase.