test(agent/agentcontainers): fix data race and flake in DuringUpWithContainerID test #21059
Conversation
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
…ntainerID test The test had two issues: 1. Data race in fakeSubAgentClient: The test helper was accessed concurrently from the API's updater loop and from API.Close() without synchronization. Added a mutex to protect shared map and slice access. 2. Missing clock advance: After nowRecreateSuccessTrap fires, CreateDevcontainer still calls RefreshContainers which needs to complete before the test checks the response. Added clock advance to match the pattern used in other similar tests. Also changed assert.NotNil to require.NotNil to prevent nil pointer dereference if the assertion fails. Fixes coder/internal#1169
mafredri
commented
Dec 2, 2025
mafredri
commented
Dec 2, 2025
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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 test had two issues:
Data race in fakeSubAgentClient: The test helper was accessed
concurrently from the API's updater loop and from API.Close()
without synchronization. Added a mutex to protect shared map
and slice access.
Missing clock advance: After nowRecreateSuccessTrap fires,
CreateDevcontainer still calls RefreshContainers which needs
to complete before the test checks the response. Added clock
advance to match the pattern used in other similar tests.
Also changed assert.NotNil to require.NotNil to prevent nil pointer
dereference if the assertion fails.
Fixes coder/internal#1169