fix(tests): give the _as cleanup guards a runtime-local token client - #109
Merged
Conversation
The teardown guard already knew it must not reuse the test's HTTP client - its connection-pool tasks belong to the test's runtime - and builds its own ApiService inside the teardown runtime for exactly that reason. What the module doc missed is that there are *two* clients. A DataHubConfig carries one of its own for the token endpoint, and the `_as` guards take a config from the test, so building an ApiService from it replaced the REST client and left the token one pointing at the test's runtime. That deadlocks rather than erroring, because the test's runtime is not busy but *blocked*: drop parks in thread::scope until teardown returns, so the one thread that could drive the connection is waiting for the thread waiting for it. A valid cached token hides it - get_api_token returns without touching the network - so it only surfaces once the token has been invalidated (a 401 clears it) or has expired. That is how one failing ACL test wedged the whole suite for 19 minutes with no output. Measured, driving the same teardown three ways: after the test runtime is dropped, ~6ms; while it is blocked, never returns; while it is blocked but with a runtime-local client, ~74ms. The regression test reproduces the blocked-runtime half faithfully (the drop runs on its own thread while the test blocks waiting for it) so a deadlock fails an assertion in 30s instead of stalling the suite silently. Verified both ways: it fails without the fix and passes with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
olavgg
approved these changes
Sep 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The teardown guard already knew it must not reuse the test's HTTP client - its connection-pool tasks belong to the test's runtime - and builds its own ApiService inside the teardown runtime for exactly that reason. What the module doc missed is that there are two clients. A DataHubConfig carries one of its own for the token endpoint, and the
_asguards take a config from the test, so building an ApiService from it replaced the REST client and left the token one pointing at the test's runtime.That deadlocks rather than erroring, because the test's runtime is not busy but blocked: drop parks in thread::scope until teardown returns, so the one thread that could drive the connection is waiting for the thread waiting for it. A valid cached token hides it - get_api_token returns without touching the network - so it only surfaces once the token has been invalidated (a 401 clears it) or has expired. That is how one failing ACL test wedged the whole suite for 19 minutes with no output.
Measured, driving the same teardown three ways: after the test runtime is dropped, ~6ms; while it is blocked, never returns; while it is blocked but with a runtime-local client, ~74ms.
The regression test reproduces the blocked-runtime half faithfully (the drop runs on its own thread while the test blocks waiting for it) so a deadlock fails an assertion in 30s instead of stalling the suite silently. Verified both ways: it fails without the fix and passes with it.