fix: eliminate failing/flaky tests in devcontainer/non-CI environments - #7906
Merged
JeffreyCA merged 2 commits intoApr 24, 2026
Merged
Conversation
- Container env tests: clear all container env vars before each subtest so ambient vars (e.g. REMOTE_CONTAINERS in devcontainer) don't cause false positives from non-deterministic map iteration. - Update action tests: inject a failing HTTP client so CheckForUpdate fails deterministically. Previously, noopCommandRunner only blocked the command runner path while CheckForUpdate used a real HTTP client. Tests passed in CI only because IsRunningOnCI() short-circuited before reaching CheckForUpdate.
JeffreyCA
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Reduces local/devcontainer flakiness in azd unit tests by isolating ambient environment effects and preventing live network calls during update checks.
Changes:
- Stabilize container-environment detection tests by clearing all container-related env vars before each subtest.
- Allow
updateActionto pass an injectable*http.Clientintopkg/update.Manager. - Make update-action tests deterministic by injecting an HTTP client whose transport always fails (no network dependency).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
cli/azd/pkg/azdext/security_validation_test.go |
Clears container env vars per subtest to avoid ambient devcontainer/Codespaces env affecting assertions. |
cli/azd/cmd/update.go |
Adds optional httpClient field on updateAction and passes it into update.NewManager. |
cli/azd/cmd/final_coverage3_test.go |
Adds a failing HTTP transport/client and injects it into updateAction tests to avoid real HTTP calls. |
Co-authored-by: Copilot <copilot@github.com>
Jon Gallant (jongio)
approved these changes
Apr 24, 2026
Jon Gallant (jongio)
left a comment
Jon Gallant (jongio)
left a comment
Member
There was a problem hiding this comment.
Clean fix for two test flakiness issues.
The map-to-slice conversion for containerEnvVars properly addresses the root cause - Go map iteration is non-deterministic, so when multiple container env vars are set (e.g. in a Codespace that's also a devcontainer), the old code could return different results between runs. The priority ordering in the slice is well-documented and sensible (CODESPACES > KUBERNETES > devcontainer).
The HTTP client injection in updateAction is the right fix for the second issue. CheckForUpdate uses HTTP (not the command runner), so noopCommandRunner alone wasn't enough to make tests deterministic outside CI. The nil-default in production preserves existing behavior.
Both test files now properly isolate env vars with t.Setenv before each subtest. Looks good.
Collaborator
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
Copilot AI
pushed a commit
that referenced
this pull request
May 21, 2026
#7906) * fix: eliminate flaky tests in devcontainer/non-CI environments - Container env tests: clear all container env vars before each subtest so ambient vars (e.g. REMOTE_CONTAINERS in devcontainer) don't cause false positives from non-deterministic map iteration. - Update action tests: inject a failing HTTP client so CheckForUpdate fails deterministically. Previously, noopCommandRunner only blocked the command runner path while CheckForUpdate used a real HTTP client. Tests passed in CI only because IsRunningOnCI() short-circuited before reaching CheckForUpdate. * Address feedback Co-authored-by: therealjohn <1501196+therealjohn@users.noreply.github.com>
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
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.
Problem
Two test suites fail when run locally in devcontainers but pass in CI:
TestIsContainerEnvironment_EnvVars— Ambient env vars likeREMOTE_CONTAINERS(set in devcontainers) interfere with assertions because Go map iteration is non-deterministic. When testingCODESPACES,ContainerRuntime()may findREMOTE_CONTAINERSfirst and return"devcontainer"instead of"codespaces".Test_UpdateAction_Run_SwitchChannel_CheckForUpdateError/Test_UpdateAction_Run_NoChannelNoConfigFlags— These expectRun()to return an error vianoopCommandRunner, butCheckForUpdateuses an HTTP client (not the command runner). In CI,IsRunningOnCI()short-circuits before reachingCheckForUpdate, masking the bug. Locally, the real HTTP call succeeds andRun()returns nil.Fix
t.Setenvbefore each subtest so only the target var is active.httpClientfield toupdateAction(nil = default) and inject afailingTransportin tests soCheckForUpdatefails deterministically without network access.