Add focused tests for wait_or_shutdown timeout and shutdown paths

[coderabbitai]

Description

Exercise both branches and edge cases of the wait_or_shutdown function to meet testing guidelines.

Problem

The wait_or_shutdown function currently lacks focused unit tests that verify both the timeout and shutdown paths work correctly.

Solution

Add a new async test module with rstest:

#[cfg(test)]
mod tests {
    use super::*;
    use rstest::rstest;
    use tokio::sync::watch;

    #[tokio::test]
    async fn returns_immediately_on_zero_secs() {
        let (_tx, mut rx) = watch::channel(());
        WorkerHooks::wait_or_shutdown(Duration::from_secs(0), &mut rx).await;
    }

    #[tokio::test]
    async fn returns_on_shutdown_before_timeout() {
        let (tx, mut rx) = watch::channel(());
        let mut rx2 = tx.subscribe();
        let fut = WorkerHooks::wait_or_shutdown(Duration::from_secs(60), &mut rx2);
        tx.send(()).expect("notify");
        fut.await;
    }
}

Location

• File: crates/comenqd/src/worker.rs
• Lines: 122-127

References

• PR: Document wait_or_shutdown helper #85
• Comment: Document wait_or_shutdown helper #85 (comment)

[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 my assessment of issue #89, I've determined this is a low criticality issue and have applied the low label accordingly.

Assessment Rationale:

Function Analysis:
The wait_or_shutdown function is a utility method in WorkerHooks that implements a timeout mechanism with early shutdown capability. It uses tokio::time::timeout to wait for a specified duration while listening for shutdown signals.

Risk Assessment:

• User Impact Risk: Low - This is a testing gap, not a functional bug
• Current Functionality: The function appears to work correctly in production
• Blast Radius: Limited - Missing tests don't directly affect runtime behavior
• Code Quality: The function logic is straightforward with clear timeout and shutdown paths

Criticality Justification:

• Not Critical/High: No immediate user-facing issues or system failures
• Not Medium: While testing is important, this specific gap doesn't indicate high-risk untested code paths
• Low: This is a code quality improvement that enhances test coverage for edge cases

The issue represents good engineering practice to improve test coverage, particularly for the timeout and shutdown scenarios, but poses minimal risk to users or system stability. The function's logic is relatively simple and the missing tests are more about completeness than catching critical bugs.

---

🏁 Script executed:

#!/bin/bash
# First, let's examine the wait_or_shutdown function in the worker.rs file
echo "=== Examining wait_or_shutdown function ==="
cat crates/comenqd/src/worker.rs | sed -n '120,130p'

echo -e "\n=== Looking for the full function implementation ==="
ast-grep --pattern $'async fn wait_or_shutdown($_) {
  $$$
}'

echo -e "\n=== Searching for any existing tests for wait_or_shutdown ==="
rg -n "wait_or_shutdown" --type=rust

echo -e "\n=== Looking at the broader context around lines 122-127 ==="
cat crates/comenqd/src/worker.rs | sed -n '115,135p'

Length of output: 1946

---

🏁 Script executed:

#!/bin/bash
# Let's examine the entire worker.rs file to understand the context better
echo "=== Full worker.rs file content ==="
cat crates/comenqd/src/worker.rs

echo -e "\n=== Checking for any existing test modules in worker.rs ==="
rg -n "mod tests" crates/comenqd/src/worker.rs

echo -e "\n=== Looking for WorkerHooks struct and its methods ==="
ast-grep --pattern $'impl WorkerHooks {
  $$$
}'

Length of output: 12065

---

🏁 Script executed:

#!/bin/bash
# Let's also check what this function is used for in the codebase
echo "=== Finding usages of wait_or_shutdown ==="
rg -n "wait_or_shutdown" --type=rust -C 3

echo -e "\n=== Looking at the overall test coverage in the crates/comenqd directory ==="
find crates/comenqd -name "*.rs" -exec sh -c 'echo "=== $1 ==="; grep -n "test\|Test" "$1"' _ {} \;

Length of output: 9767

---

🏁 Script executed:

#!/bin/bash
# Let's examine the GitHub issue to understand better
echo "=== Checking current labels on the issue ==="
gh issue view 89 --json labels,title,body,state

echo -e "\n=== Looking at repository labels to see available criticality labels ==="
gh label list | grep -E "(critical|high|medium|low|documentation)"

Length of output: 1676

---

🏁 Script executed:

#!/bin/bash
# Apply the appropriate labels based on my assessment
# This appears to be a low criticality testing improvement issue

echo "=== Applying 'low' criticality label ==="
gh issue edit 89 --add-label "low"

echo -e "\n=== Verifying label was applied ==="
gh issue view 89 --json labels

Length of output: 258

---

@leynos Based on my comprehensive assessment of issue #89, I've determined this is a low criticality issue and have successfully applied the low label.

Assessment Rationale:

Function Analysis:
The wait_or_shutdown function is a utility method that implements a cooldown mechanism in the worker loop. It uses tokio::select! to wait for either a timeout or shutdown signal, serving as a rate-limiting mechanism between comment posting attempts.

Current Usage:

• Called twice in the main worker loop during cooldown periods
• Already has comprehensive documentation with working examples
• Function logic is straightforward with only two execution paths

Risk Assessment:

• User Impact Risk: Minimal - This is purely a test coverage gap, not a functional defect
• Current Functionality: The function works correctly in production (evidenced by existing integration tests)
• Blast Radius: Limited - Missing unit tests don't affect runtime behavior
• Existing Coverage: The function is already exercised through integration tests in the worker behavior tests

Test Gap Analysis:

• No dedicated unit tests exist for the wait_or_shutdown function specifically
• Integration tests cover the function indirectly through worker behavior
• The proposed tests would verify edge cases (zero timeout, early shutdown) more explicitly

Criticality Justification:
This represents good engineering practice to improve test coverage completeness, but poses minimal risk to users or system stability. The function's simple logic and existing integration test coverage reduce the likelihood of hidden bugs.

The label has been successfully applied to the issue.