Skip to content

Make provider test fixtures wait for their lifecycle - #6

Merged
numinousmuses merged 1 commit into
masterfrom
fix/provider-test-lifecycle
Sep 6, 2026
Merged

numinousmuses merged 1 commit into
masterfrom
fix/provider-test-lifecycle

Conversation

@numinousmuses

Copy link
Copy Markdown

Keep persisted and live reconnect sessions distinct in the online-delete fixture, and wait for failover-provider scripts to exit before test teardown. This removes a raced ownership overwrite and logging after testing.T completion. Production code and the existing response assertions remain unchanged.

The unchanged delete fixture failed 6 of 20 focused repetitions. A new real HTTP/provider regression holds an active script and fails with the old cleanup because it returns before the script exits. The repaired fixtures pass 20 focused repetitions under the race detector, including both reasoning fallback branches. Forge CI verifies the complete merged coordinator source independently.

flowchart LR
  subgraph Before
    B1[TestDeleteMyProvider_OnlineConflict409] --> B2[Seed and register the same session ID]
    B2 --> B3[Async persistence overwrites ownership]
    B3 --> B4[403 instead of required409]
    B5[failoverProvider.close] --> B6[Close socket and return]
    B6 --> B7[Script can log after test teardown]
  end
Loading
flowchart LR
  subgraph After
    A1[TestDeleteMyProvider_OnlineConflict409] --> A2[Stored and live sessions share serial but have distinct IDs]
    A2 --> A3[Ownership remains available]
    A3 --> A4[Online conflict returns409]
    A5[failoverProvider.close] --> A6[Cancel run context and close socket]
    A6 --> A7[Wait for fp.done before teardown]
    A8[TestFailoverProviderCleanupJoinsActiveScript] --> A7
  end
Loading

closeNow remains nonjoining because scripts use it to simulate a connection crash. The cleanup path still joins the goroutine afterward. This is a separate test-reliability change found during review of the streaming repair.

@numinousmuses
numinousmuses marked this pull request as ready for review September 6, 2026 02:06
@numinousmuses
numinousmuses merged commit 9fc68d9 into master Sep 6, 2026
2 checks passed
@numinousmuses

Copy link
Copy Markdown
Author

Numinous Forge · Change and verification

Result: test reliability fixes, merged into this fork. Production code and the existing response assertions are unchanged.

What was wrong: A reconnect fixture reused the same session ID for stored and live sessions, allowing background persistence to overwrite ownership. Separately, a provider test helper could return from cleanup while its script was still running.

What changed: Use distinct session IDs for the same device and wait for active provider scripts to exit during cleanup.

What we proved:

  • The original delete fixture failed 6 of 20 focused repetitions.
  • A new cleanup regression fails with the old helper because cleanup returns before the active script exits.
  • The repaired fixtures passed 20 focused repetitions under the race detector. Separate PR CI passed coordinator, protocol, and documentation checks on the merged source.

Review focus: Keep the original ownership assertions intact. Normal cleanup must join the script; the explicit connection-crash helper must remain usable from inside the script without waiting on itself.

Read the test changes

Review summary for 9a2a01b34c22. Live task and CI progress are reported separately.

@numinousmuses numinousmuses added the trigger:ci Forge was triggered by a contributor PR or new commit. label Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

trigger:ci Forge was triggered by a contributor PR or new commit.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant