Fix flaky persistent subscription nack tests

[w1am]

Problem

Both nack tests in subscribeToPersistentSubscriptionToStream.test.ts were flaky.

They used finish-test as the sole stop signal. But finish-test is delivered in the first pass, while retry-nacked events are redelivered asynchronously with no ordering guarantee. If finish-test won the race, the test stopped early (~41 handler calls instead of 61) and failed.

Fix

Stop only once finish-test is seen and all retry events have been redelivered and acked. Order-independent, so no race. Assertions unchanged.

[qodo-code-review]

PR Summary by Qodo

Stabilize persistent subscription nack tests by waiting for retry redeliveries
🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Description

• Prevent premature test termination when finish event races retry redeliveries.
• Track finish event and acked retry redeliveries before resolving/breaking.
• Keep existing assertions; make completion order-independent and deterministic.

Diagram

flowchart TD
  A([Jest test]) --> B["Subscribe persistent"] --> C{{EventStoreDB}} --> D["Async retry"] --> E["Handler loop"] --> F{"Finish seen & retries acked?"} --> G([Resolve break])
  E -->|"Ack/Nack"| C

  subgraph Legend
    direction LR
    _test([Test control]) ~~~ _ext{{External system}} ~~~ _dec{"Decision node"}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Wait for expected call count (polling) instead of finish marker

• ➕ Removes reliance on a synthetic finish event
• ➕ Directly asserts the observable behavior the test cares about
• ➖ Can be more timing-sensitive if call counts fluctuate temporarily
• ➖ Often requires carefully tuned timeouts/retries to avoid new flake modes

2. Make redelivery deterministic via server/subscription settings

• ➕ Reduces nondeterminism at the source (retry cadence/ordering)
• ➕ Potentially simplifies test termination logic
• ➖ May not be feasible or exposed via the client API
• ➖ Couples tests to configuration details and may reduce coverage realism

Recommendation: The PR’s approach (gating termination on both the finish marker and the full set of redelivered retry events being acked) is the most robust while keeping existing assertions and behavior intact. Alternatives either reintroduce timing-based polling flakiness or require deeper configuration control that may not be available or desirable for these integration-style tests.

Files changed (1) +23 / -2

Bug fix (1) +23 / -2

subscribeToPersistentSubscriptionToStream.test.tsMake nack tests order-independent by tracking finish and acked retry redeliveries +23/-2

Make nack tests order-independent by tracking finish and acked retry redeliveries

• Updates both the event-emitter and async-iterator nack tests to avoid exiting when the finish event arrives before asynchronous retry redeliveries. Adds tracking for finishSeen and a set of acked retry-event IDs, and only resolves/breaks once all expected retries have been redelivered and acked.

packages/test/src/persistentSubscription/subscribeToPersistentSubscriptionToStream.test.ts

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review