Skip to content

Queue layer has no way to test cancellation windows #60

Description

@Jake-Moore

The gap

Every defect found while reviewing #57 that could be reproduced deterministically got a
regression test. Every defect that was a scheduler race got none. That split is not a coincidence:
the suite has no way to control scheduling.

  • UpdateQueue builds its own scope on Dispatchers.IO (UpdateQueue.kt), and
    UpdateQueueManager hardcodes Dispatchers.IO in its coroutineContext. Neither is injectable,
    so a test cannot decide when a coroutine runs.
  • kotlinx-coroutines-test is already declared as a test dependency in both core-api and
    plugin-api, but runTest, TestScope and TestDispatcher appear nowhere in the codebase.
  • There is one unit test, test/unit/TestConnectionSequence. Every other test in core-api runs
    against a real MongoDB container, so probing queue internals means creating a document first.

What that cost

Four defects fixed in #57 shipped without a regression test, three of which become straightforward
with a controllable dispatcher:

Defect Reproducible with an injectable dispatcher
A backpressure retry cancelled before its body started left the caller's deferred dangling Yes. Launch, cancel, never run the scheduler.
The idle sweep removed a queue from the map, then failed to shut it down when its launched coroutine was cancelled before starting Yes, same mechanism.
An enqueue could create a queue after shutdown had taken its snapshot Yes, by interleaving both on one test dispatcher.
completedCount was incremented after the deferred it belonged to was completed No. The two statements are adjacent and non-suspending, so there is no scheduling point to interpose. This one stays a structural guarantee.

Proposed work

  1. Accept a CoroutineContext or CoroutineDispatcher in UpdateQueue and UpdateQueueManager,
    defaulting to Dispatchers.IO so no caller changes.
  2. Add a unit-level suite for the queue layer using StandardTestDispatcher, covering the three
    rows above, plus the invariant they are all instances of: every deferred handed to a caller is
    eventually completed, and never with a CancellationException.
  3. Assert the metrics hooks. Nothing asserted any metric before Bound update waits by queue progress rather than by ping #57, which is how an undercount on
    onDatabaseUpdateFail nearly shipped unnoticed.

Not proposed

Randomised or stress-based race hunting. A test that only fails sometimes is worse than no test,
because a red run stops meaning anything.

Activity

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

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions