Skip to content

Test WorkerHooks::notify_drained_if_empty directly to catch negation mutants #153

Open
Open
Test WorkerHooks::notify_drained_if_empty directly to catch negation mutants#153
Labels
mediumRoadmap items to schedule within the current quarter. Clear scope, normal review cycles.testingTest coverage, test infrastructure, and verification tooling work.

Description

@leynos
leynos
opened on Jul 20, 2026

Context

Scheduled mutation run 29495843050 (2026-07-16, first green run after the baseline fix in #140) exercised 22 mutants across crates/comenqd/src/listener.rs, crates/comenqd/src/logging.rs, and crates/comenqd/src/worker.rs: 4 caught, 3 missed, 4 timed out, 11 unviable.

Two of the three missed mutants are negations of the same predicate in WorkerHooks::notify_drained_if_empty:

crates/comenqd/src/worker.rs:94:25: delete ! in WorkerHooks::notify_drained_if_empty
crates/comenqd/src/worker.rs:96:26: delete ! in WorkerHooks::notify_drained_if_empty

#[cfg(any(test, feature = "test-support"))]
fn notify_drained_if_empty(&self, queue_path: &Path) -> std::io::Result<()> {
    if let Some(n) = &self.drained {
        // Ignore sentinel files left by the queue implementation and
        // consider the directory empty when no other files remain.
        let empty = !stdfs::read_dir(queue_path)?
            .filter_map(Result::ok)
            .any(|e| !is_metadata_file(e.file_name()));
        if empty {
            n.notify_one();
        }
    }
    ...
}

Analysis

This helper is cfg-gated to test and test-support builds, and exists specifically so integration tests can wait for the queue to drain. Because it is test synchronisation machinery, an inverted empty calculation would not fail functionally — it would either notify on every poll (masking a real drain) or never notify (causing waiters to hang or fall back to a timeout), which is exactly the kind of failure that manifests as flaky or slow tests rather than a clean assertion failure. That is consistent with it going undetected by the current suite: nothing exercises the predicate directly, only indirectly through tests that use the notification as one of several synchronisation signals.

Proposed next step

Add a direct unit test for notify_drained_if_empty that covers: an empty directory (should notify), a directory containing only sentinel/metadata files (should notify), and a directory containing at least one non-metadata file (should not notify). This directly exercises the !...any(...) predicate and its negation, independent of the higher-level integration tests that currently rely on it only incidentally.

Activity

  1. coderabbitai commented on Jul 20, 2026

    @coderabbitai
    coderabbitaiBot
    on Jul 20, 2026 – with coderabbitai
    Contributor
    🔗 Related PRs

    leynos/whitaker#235 - Implement rstest helper call-site collection (8.2.2) [open]
    leynos/netsuke#410 - Adopt the Whitaker Dylint suite in the lint gate and CI [merged]
    leynos/diesel-cte-ext#99 - Skip mutation of the non-postgres SEARCH clause gate (#92) [merged]
    leynos/pg-embed-setup-unpriv#186 - Restore dev-worker coverage ratchet with unprivileged unit tests [merged]
    leynos/wireframe#612 - Kill mutation-testing survivors and widen scaffolding excludes (#566, #567, #569, #570, #593, #595, #598) [merged]

    📝 Issue Planner

    Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.

    🧪 Issue enrichment is currently in open beta.

    You can configure auto-planning by selecting labels in the issue_enrichment configuration.

    To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

    issue_enrichment:
  auto_enrich:
    enabled: false

    💬 Have feedback or questions? Drop into our discord!

  2. coderabbitai
    added
    mediumRoadmap items to schedule within the current quarter. Clear scope, normal review cycles.
    testingTest coverage, test infrastructure, and verification tooling work.
    on Jul 20, 2026

  3. leynos commented on Oct 20, 2026

    @leynos
    leynos
    on Oct 20, 2026
    OwnerAuthor

    @CodeRabbit plan

  4. coderabbitai commented on Oct 20, 2026

    @coderabbitai
    coderabbitaiBot
    on Oct 20, 2026 – with coderabbitai
    Contributor

    Coding Plan

    Summary

    • Retarget the ticket at the current drain path: the claim_next_due() None branch in run_worker and the notify_drained hook. Do not restore deleted helpers.
    • Add one narrow test module under worker/tests/scheduling/. It tests hook delivery directly. It drives the real worker for the empty, metadata-only-after-drain and deferred-entry scenarios.
    • Make every assertion deterministic. Use paused Tokio time, join the worker before inspection, and poll Notify permits with Waker::noop() instead of timeouts.
    • Validate with targeted tests, a focused cargo mutants run on worker.rs, and the full make gates.

    File-level changes

    File Change Intent
    crates/comenqd/src/worker/tests/scheduling.rs Add mod drained; Include the new child module. Give it access to existing scheduling helpers.
    crates/comenqd/src/worker/tests/scheduling/drained.rs New file Add a permit-polling helper, hook-delivery tests and worker drain-decision tests (empty, metadata only after drain, deferred).
    crates/comenqd/src/worker.rs No change Keep production behaviour as is. Leave the drained doc wording ("queue is empty") untouched.

    Out of scope: restoring notify_drained_if_empty or is_metadata_file. Changing the drained trigger semantics. Rewording hook documentation. Testing the claimed-entry rule at the worker boundary. Fixing the other missed mutants from run 29495843050.

    Prompts for follow-up agents

    Implement the drained-hook tests for comenqd.

1. Read crates/comenqd/src/worker.rs. Locate WorkerHooks::notify_drained,
   WorkerControl::notify_drained and the claim_next_due() None branch in
   run_worker. Confirm that shutdown stops the worker in both the idle and
   the deferred-waiting states.
2. Create crates/comenqd/src/worker/tests/scheduling/drained.rs with a //!
   module comment. Add `mod drained;` to
   crates/comenqd/src/worker/tests/scheduling.rs.
3. Add a helper that polls a fresh notify.notified() future once with
   Waker::noop() and reports Ready or Pending. Use the pattern in
   worker/tests.rs:299-321.
4. Add rstest cases for direct hook delivery: drained Some -> exactly one
   permit; drained None -> no panic.
5. Add rstest cases under #[tokio::test(start_paused = true)] that drive the
   real worker: empty queue -> drained; one posted entry plus a stray .tmp
   in entries/ -> drained and metadata present; deferred entry -> await the
   waiting hook, then no drained permit. Shut down and join the worker before
   you poll the drained permit.
6. Do not change production code. Do not use timeouts for negative
   assertions. Use .expect(), not .unwrap(). Keep the file under 400 lines.
7. Run the targeted tests, cargo mutants on crates/comenqd/src/worker.rs,
   then make fmt, make check-fmt, make lint and make test.

    Write the pull request description.

State that WorkerHooks::notify_drained_if_empty and is_metadata_file no
longer exist in the current checkout. State that run_worker now signals
drained when claim_next_due() returns None, and that the store excludes
metadata by directory layout. List the drained-path mutants found by
cargo mutants --list, and mark each one as caught or missed after the new
tests. Reference mutation run 29495843050.
    Design Choices
    Design Choice 1: How to treat the ticket's reference to a helper that no longer exists

    Options Considered:

    1. Restore notify_drained_if_empty and is_metadata_file, then test them as the ticket describes.
    2. Close the ticket as superseded with no code change.
    3. Keep the ticket's intent and test the current drain decision and hook delivery directly. Map the ticket's three scenarios to their current equivalents.

    Chosen Option: 3

    Rationale: The ticket's goal is to catch silent drain-signal mutants, and the current notify_drained path has the same coverage gap. Git history has one squashed commit, so the removal of notify_drained_if_empty cannot be traced. The current worker uses claim_next_due() instead of a filesystem scan.

    Design Choice 2: How to make the drain decision directly testable

    Options Considered:

    1. Extract a private predicate function from run_worker and unit-test it.
    2. Drive the real worker loop under paused Tokio time. Assert the drained permit state after the worker stops.

    Chosen Option: 2

    Rationale: This option needs no production refactor, and it exercises the real claim_next_due() boundary that decides the drain. The decision is inline in run_worker, and no standalone predicate exists.

    Design Choice 3: How to cover the ticket's "metadata-only directory" scenario

    Options Considered:

    1. Hand-write root-level metadata files with synthetic content before the store opens.
    2. Produce real metadata by posting one entry through the worker. Add only a stray .tmp file in entries/ by hand.

    Chosen Option: 2

    Rationale: Real operations write valid metadata contents, and the store does not fail on unparseable files. The current store excludes metadata by directory layout, not by filename.

    Design Choice 4: Whether to cover the "claimed entry" case at the worker boundary

    Options Considered:

    1. Add a worker-level test that pins the claimed-entry behaviour.
    2. Exclude it. Rely on store/tests/claims.rs for the store-level rule.

    Chosen Option: 2

    Rationale: Worker-level setup interacts with recovery and adds no extra mutant coverage. It would also pin semantics that differ from the documented "queue is empty" wording. Note: next_due() returns None when any entry holds a claim token, so the drained hook fires while entries still exist. Startup recovery can also release claims before the first loop.

    💡 User Tips

    Regenerate the plan with different choices with @coderabbitai <feedback>.

    Implementation Steps

    Phase 1: Verify the target and the mutant set

    Confirm the current code shape and list the mutants that the new tests must kill. This prevents tests aimed at stale symbols.

    Task 1.1: Confirm the drain path and enumerate the worker mutants

    Establish the exact mutants that survive today on the drained path.

    • Read crates/comenqd/src/worker.rs around WorkerHooks::notify_drained, WorkerControl::notify_drained and the claim_next_due() branch in run_worker.
    • Confirm that the shutdown arm of the tokio::select! stops the worker in both the "no due entry" state and the "deferred entry, waiting" state. The negative test depends on a clean worker exit.
    • Run cargo mutants --list --file crates/comenqd/src/worker.rs. Record the mutants on notify_drained (body replaced with ()) and on the None branch.
    • Record the result in the pull request description. State that notify_drained_if_empty and is_metadata_file no longer exist and that the tests target their current replacements.
    🤖 Prompt for AI agents 
    Verify the current code shape before you write tests. Goal: list the
drained-path mutants that the new tests must kill.

- Read `crates/comenqd/src/worker.rs` around `WorkerHooks::notify_drained`,
`WorkerControl::notify_drained` and the `claim_next_due()` branch in
`run_worker`.
- Confirm that the shutdown arm of the `tokio::select!` stops the worker in the
"no due entry" state and in the "deferred entry, waiting" state.
- Run `cargo mutants --list --file crates/comenqd/src/worker.rs`.
- Record the mutants on `notify_drained` (body replaced with `()`) and on the
`None` branch.
- Note in the pull request description that `notify_drained_if_empty` and
`is_metadata_file` no longer exist and that the tests target their current
replacements.
- Do not change production code.

    Phase 2: Add direct drained-notification tests

    Add one narrow test module with two layers. The first layer checks hook delivery directly. The second layer checks the worker's drain decision for each scenario from the ticket. All assertions poll a Notify permit with Waker::noop() after the operation finishes. No assertion uses sleeps or negative timeouts.

    Task 1: Task 2.1: Create the test module and permit-polling helper

    Provide the module skeleton and one shared deterministic assertion.

    • Create crates/comenqd/src/worker/tests/scheduling/drained.rs. Start it with //! documentation that explains that the module guards the drained hook against silent mutants.
    • Declare mod drained; in crates/comenqd/src/worker/tests/scheduling.rs next to the existing mod mutations;. Placement under scheduling gives access to worker_with_successful_posts, pending_entries, the request builders, the clock helpers and the hook-wait helper.
    • Add a small helper that pins a fresh notify.notified() future, polls it once with Waker::noop(), and returns whether a permit was ready. Follow the polling pattern in worker/tests.rs:299–321.
    • Assert "notified" as "first poll is Ready, and a second fresh poll is Pending". This proves that exactly one permit was buffered.
    • Use .expect() with clear messages instead of .unwrap(). Keep the file under 400 lines.
    Task 2: Task 2.2: Test hook delivery directly

    Kill mutants that empty WorkerHooks::notify_drained or WorkerControl::notify_drained.

    • Build WorkerHooks { drained: Some(notify.clone()), ..Default::default() }, wrap it with the private WorkerControl::new(shutdown, hooks), and call control.notify_drained().
    • Assert with the Task 2.1 helper that exactly one permit is ready.
    • Add a case with drained: None (via WorkerControl::without_hooks). Assert that the call returns without panic and that an unrelated Notify holds no permit.
    • Use rstest #[case] parameters for the Some and None cases, as AGENTS.md requires for repeated conditions.
    Task 3: Task 2.3: Test the worker drain decision across queue states

    Map the ticket's three scenarios to the current claim_next_due() boundary.

    • Use #[tokio::test(start_paused = true)] and the injected clock, following worker/tests/scheduling.rs.
    • Define a scenario enum with rstest #[case] values. Give each case its setup, the hook to await, and the expected drained result:
      • Empty queue: start the worker on a fresh temp_config queue. Await drained through the existing hook-wait helper. Expect notified.
      • Metadata only after drain: enqueue one due request through dispatch_request(Request::Put(...)). Write a stray .tmp file into the entries/ directory. Run worker_with_successful_posts. Await drained. Assert that pending_entries is empty and that root-level metadata (for example history.jsonl or last_post) exists. Expect notified. This case kills a mutant that fires drained only when the queue directory is empty.
      • Deferred entry: enqueue one request with a future ETA. Await the waiting hook instead of drained. Expect not notified.
    • After the awaited hook fires, send shutdown and join the worker handle. Then call the Task 2.1 helper on the drained notify. Inspect the permit only after the worker task has finished. The negative result is then final.
    • For the deferred case, assert that pending_entries still holds the entry. This proves that the queue was not empty at the observation point.
    • Do not use tokio::time::timeout for negative assertions. Use the real-thread watchdog only through the existing hook-wait helper.
    🤖 Prompt for AI agents 
    Add one narrow test module with two layers: direct hook delivery, and worker
drain-decision scenarios. Goal: kill silent drained-signal mutants without
sleeps or negative timeouts.

Module setup:
- Create `crates/comenqd/src/worker/tests/scheduling/drained.rs` with `//!` docs
that explain it guards the drained hook against silent mutants.
- Add `mod drained;` in `crates/comenqd/src/worker/tests/scheduling.rs` next to
`mod mutations;`.
- Reuse `worker_with_successful_posts`, `pending_entries`, request builders,
clock helpers and the hook-wait helper.
- Keep the file under 400 lines. Use `.expect()` with clear messages. Do not use
`.unwrap()`.

Permit helper:
- Pin a fresh `notify.notified()` future and poll it once with `Waker::noop()`.
Follow `worker/tests.rs:299–321`.
- Treat "notified" as: first poll `Ready` and a second fresh poll `Pending`.

Hook delivery tests (use `rstest` `#[case]` for Some and None):
- Build `WorkerHooks { drained: Some(notify.clone()), ..Default::default() }`.
Wrap it with `WorkerControl::new(shutdown, hooks)`. Call
`control.notify_drained()`. Assert exactly one permit.
- For `drained: None` use `WorkerControl::without_hooks`. Assert no panic and no
permit on an unrelated `Notify`.

Worker scenario tests:
- Use `#[tokio::test(start_paused = true)]`, the injected clock and `rstest`
`#[case]` with a scenario enum.
- Empty queue: start the worker on a fresh `temp_config` queue. Await `drained`
with the hook-wait helper. Expect notified.
- Metadata only after drain: enqueue one due request with
`dispatch_request(Request::Put(...))`. Write a stray `.tmp` file in `entries/`.
Run `worker_with_successful_posts`. Await `drained`. Assert `pending_entries` is
empty and root-level metadata (`history.jsonl` or `last_post`) exists. Expect
notified.
- Deferred entry: enqueue one request with a future ETA. Await the `waiting`
hook. Expect not notified. Assert `pending_entries` still holds the entry.
- After the awaited hook fires, send shutdown and join the worker handle. Poll
the `drained` permit only after the join.
- Do not use `tokio::time::timeout` for negative assertions. Use the real-thread
watchdog only through the hook-wait helper.
- Do not change production code.

    Phase 3: Validate

    Run the targeted tests, the mutation check and the repository gates.

    Task 1: Task 3.1: Run targeted tests and mutation testing

    Prove that the new tests pass and kill the drained-path mutants.

    • Run cargo test -p comenqd --lib worker::tests::scheduling::drained. Repeat with --all-features.
    • Run cargo mutants --workspace --all-features --test-workspace=true --file crates/comenqd/src/worker.rs.
    • Confirm that the notify_drained mutants recorded in Task 1.1 are now caught. Record any mutants that still survive in the pull request description. Do not widen scope to fix them.
    Task 2: Task 3.2: Run repository gates

    Satisfy the AGENTS.md pre-commit requirements.

    • Run make fmt, make check-fmt, make lint and make test.
    • Fix lint findings in the new module. Do not add lint suppressions.
    🤖 Prompt for AI agents 
    Validate the new tests, the mutation result and the repository gates. Goal:
prove the tests pass and kill the drained-path mutants.

- Run `cargo test -p comenqd --lib worker::tests::scheduling::drained`. Repeat
with `--all-features`.
- Run `cargo mutants --workspace --all-features --test-workspace=true --file
crates/comenqd/src/worker.rs`.
- Confirm that the `notify_drained` mutants from Phase 1 are caught. Record
surviving mutants in the pull request description. Do not widen scope to fix
them.
- Run `make fmt`, `make check-fmt`, `make lint` and `make test`.
- Fix lint findings in the new module. Do not add lint suppressions.
    Research

    The ticket names WorkerHooks::notify_drained_if_empty and is_metadata_file. Neither symbol exists in the current checkout. Retarget the tests at the current drain decision in run_worker (claim_next_due() returns None) and at the notify_drained hook path. Do not change production behaviour.

    crates/comenqd/src/worker.rs defines WorkerHooks (cfg-gated to test or test-support) with enqueued, idle, drained and waiting fields of type Option<Arc<Notify>>. The private WorkerHooks::notify_drained calls notify_one() when the drained hook is set. WorkerControl::notify_drained forwards to it in test builds and is a no-op otherwise. run_worker calls control.notify_drained() when queue.claim_next_due() returns None, then waits on shutdown or queue.changed().

    The store separates queue entries from metadata structurally. It scans only entries/*.json. Root-level last_post, last_attempt, history.jsonl and completion, plus .tmp siblings, are never pending entries.

    Worker unit tests live under crates/comenqd/src/worker/tests/. Child modules can access private ancestor items. worker/tests/scheduling.rs holds helpers such as worker_with_successful_posts, pending_entries, request builders, an injected clock, and a paused-time hook-wait helper with a watchdog.

    No current test awaits the drained hook. Existing scheduling tests create a drained hook but never wait on it. A mutant that empties notify_drained therefore survives.

    🚀 Next Steps

    🤖 All AI agent prompts combined 
    Task: 1

Verify the current code shape before you write tests. Goal: list the
drained-path mutants that the new tests must kill.

- Read `crates/comenqd/src/worker.rs` around `WorkerHooks::notify_drained`,
`WorkerControl::notify_drained` and the `claim_next_due()` branch in
`run_worker`.
- Confirm that the shutdown arm of the `tokio::select!` stops the worker in the
"no due entry" state and in the "deferred entry, waiting" state.
- Run `cargo mutants --list --file crates/comenqd/src/worker.rs`.
- Record the mutants on `notify_drained` (body replaced with `()`) and on the
`None` branch.
- Note in the pull request description that `notify_drained_if_empty` and
`is_metadata_file` no longer exist and that the tests target their current
replacements.
- Do not change production code.
===============================================================================

Task: 2

Add one narrow test module with two layers: direct hook delivery, and worker
drain-decision scenarios. Goal: kill silent drained-signal mutants without
sleeps or negative timeouts.

Module setup:
- Create `crates/comenqd/src/worker/tests/scheduling/drained.rs` with `//!` docs
that explain it guards the drained hook against silent mutants.
- Add `mod drained;` in `crates/comenqd/src/worker/tests/scheduling.rs` next to
`mod mutations;`.
- Reuse `worker_with_successful_posts`, `pending_entries`, request builders,
clock helpers and the hook-wait helper.
- Keep the file under 400 lines. Use `.expect()` with clear messages. Do not use
`.unwrap()`.

Permit helper:
- Pin a fresh `notify.notified()` future and poll it once with `Waker::noop()`.
Follow `worker/tests.rs:299–321`.
- Treat "notified" as: first poll `Ready` and a second fresh poll `Pending`.

Hook delivery tests (use `rstest` `#[case]` for Some and None):
- Build `WorkerHooks { drained: Some(notify.clone()), ..Default::default() }`.
Wrap it with `WorkerControl::new(shutdown, hooks)`. Call
`control.notify_drained()`. Assert exactly one permit.
- For `drained: None` use `WorkerControl::without_hooks`. Assert no panic and no
permit on an unrelated `Notify`.

Worker scenario tests:
- Use `#[tokio::test(start_paused = true)]`, the injected clock and `rstest`
`#[case]` with a scenario enum.
- Empty queue: start the worker on a fresh `temp_config` queue. Await `drained`
with the hook-wait helper. Expect notified.
- Metadata only after drain: enqueue one due request with
`dispatch_request(Request::Put(...))`. Write a stray `.tmp` file in `entries/`.
Run `worker_with_successful_posts`. Await `drained`. Assert `pending_entries` is
empty and root-level metadata (`history.jsonl` or `last_post`) exists. Expect
notified.
- Deferred entry: enqueue one request with a future ETA. Await the `waiting`
hook. Expect not notified. Assert `pending_entries` still holds the entry.
- After the awaited hook fires, send shutdown and join the worker handle. Poll
the `drained` permit only after the join.
- Do not use `tokio::time::timeout` for negative assertions. Use the real-thread
watchdog only through the hook-wait helper.
- Do not change production code.
===============================================================================

Task: 3

Validate the new tests, the mutation result and the repository gates. Goal:
prove the tests pass and kill the drained-path mutants.

- Run `cargo test -p comenqd --lib worker::tests::scheduling::drained`. Repeat
with `--all-features`.
- Run `cargo mutants --workspace --all-features --test-workspace=true --file
crates/comenqd/src/worker.rs`.
- Confirm that the `notify_drained` mutants from Phase 1 are caught. Record
surviving mutants in the pull request description. Do not widen scope to fix
them.
- Run `make fmt`, `make check-fmt`, `make lint` and `make test`.
- Fix lint findings in the new module. Do not add lint suppressions.
    💡 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 design choice 1 go ahead with option 3 and replan.

    Start Coding →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    mediumRoadmap items to schedule within the current quarter. Clear scope, normal review cycles.testingTest coverage, test infrastructure, and verification tooling work.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions