Self-review of #2755 found two tests that didn't actually exercise the
production code path:
- TestNamespaceCleanupFn_NamespaceFormat asserted
"workspace:" + "abc-123" == "workspace:abc-123" — a compile-time
invariant, not runtime behavior. Provided no protection if the closure
in Bundle.NamespaceCleanupFn ever stopped using that prefix.
- TestNamespaceCleanupFn_FailureLogsButReturns built a *parallel*
cleanup closure inline with errors.New, then invoked the parallel
closure. The production closure was never exercised. A regression
in NamespaceCleanupFn (e.g. forgetting the deferred recover, calling
the plugin without nil-check) would still pass this test.
Replaced both with real integration:
- TestNamespaceCleanupFn_HitsPluginAtCorrectNamespace spins up
httptest.Server, points MEMORY_PLUGIN_URL at it, calls Build(),
invokes the production closure, and asserts the server actually
saw DELETE /v1/namespaces/workspace:abc-123.
- TestNamespaceCleanupFn_PluginErrorDoesNotPanic exercises the
failure path for real: server returns 500 on DELETE, closure must
log and return without propagating. defer-recover is belt-and-
suspenders since production calls this from a for-loop in
workspace_crud.go that has no recover.
Couldn't ship with #2755 because the merge queue locks the branch
once enqueued. Following up now that #2755 is merged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
707e4d7342