test: fix unknown CI failures - #7714
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes flaky CI “(unknown)” test failures by preventing interleaved/partial stdout from confusing go test -json/gotestsum output parsing, and by making generated Fig completion output consistently newline-terminated.
Changes:
- Add a shared captured test console helper that buffers stdout/stderr and only prints on test failure.
- Update repository interactive tests to use the captured console and
t.Context()instead of printing extra newlines to stdout. - Ensure
azd completion figemits a trailing newline; update testdata and tests accordingly.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/internal/repository/test_console_test.go | Adds a buffered test console that only logs captured output on failure. |
| cli/azd/internal/repository/infra_confirm_test.go | Switches interactive tests to use the captured console and test context. |
| cli/azd/internal/repository/detect_confirm_test.go | Switches interactive tests to use the captured console and test context. |
| cli/azd/internal/repository/app_init_test.go | Switches interactive tests to use the captured console and test context. |
| cli/azd/cmd/testdata/TestFigSpec.ts | Adds trailing newline to keep generated spec output shell/test friendly. |
| cli/azd/cmd/completion_test.go | Captures command stdout and asserts the Fig spec content includes a trailing newline. |
| cli/azd/cmd/completion.go | Writes the Fig spec to stdout with Fprintln to enforce a trailing newline. |
Copilot's findings
- Files reviewed: 7/7 changed files
- Comments generated: 3
jongio
left a comment
There was a problem hiding this comment.
Two separate sources of gotestsum "unknown" failures, both fixed correctly.
The fmt.Fprintln change is exact - the TypeScript template doesn't end with a newline, so this adds precisely one. The snapshot update matches.
newCapturedTestConsole is the right call. Centralizing the setup removes three copies of boilerplate, captures output so it doesn't pollute parallel test output, and only surfaces it when a test actually fails. The t.Helper() is there, cleanup is conditional on failure - nothing to improve.
The removed fmt.Println() calls were load-bearing workarounds; they're no longer needed once output is captured. Good to see them gone.
|
/check-enforcer override |
|
@copilot resolve the merge conflicts in this pull request |
1 similar comment
|
@copilot resolve the merge conflicts in this pull request |
Ensure `azd completion fig` ends with a trailing newline so gotestsum/JUnit does not turn a passing test into an `(unknown)` failure. Capture interactive console output in internal/repository tests and emit it only when a test fails, avoiding stdout interleaving like `=== FAIL: internal/repository TestInitializer_PromptIfNonEmpty (unknown)`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
4e6c699 to
fcdec0f
Compare
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
… creds Test_CLI_Up_Down_ContainerApp_RemoteBuild_MultiService called recording.Start(t) before its skip check. recording.Start does t.Fatalf when the cassette is missing, so CI without cassettes AND without AZURE_TENANT_ID hard-failed instead of skipping. Fix: check cassette existence + AZURE_TENANT_ID before recording.Start, skip cleanly if both absent. Keep the original post-Start nil-session skip as defensive guard. (The companion completion_test.go phantom-failure fix from this same commit was superseded by upstream PR #7714 during rebase, which adds a trailing newline in the renderer plus stdout capture.) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… creds Test_CLI_Up_Down_ContainerApp_RemoteBuild_MultiService called recording.Start(t) before its skip check. recording.Start does t.Fatalf when the cassette is missing, so CI without cassettes AND without AZURE_TENANT_ID hard-failed instead of skipping. Fix: check cassette existence + AZURE_TENANT_ID before recording.Start, skip cleanly if both absent. Keep the original post-Start nil-session skip as defensive guard. (The companion completion_test.go phantom-failure fix from this same commit was superseded by upstream PR #7714 during rebase, which adds a trailing newline in the renderer plus stdout capture.) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… creds Test_CLI_Up_Down_ContainerApp_RemoteBuild_MultiService called recording.Start(t) before its skip check. recording.Start does t.Fatalf when the cassette is missing, so CI without cassettes AND without AZURE_TENANT_ID hard-failed instead of skipping. Fix: check cassette existence + AZURE_TENANT_ID before recording.Start, skip cleanly if both absent. Keep the original post-Start nil-session skip as defensive guard. (The companion completion_test.go phantom-failure fix from this same commit was superseded by upstream PR #7714 during rebase, which adds a trailing newline in the renderer plus stdout capture.) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… creds Test_CLI_Up_Down_ContainerApp_RemoteBuild_MultiService called recording.Start(t) before its skip check. recording.Start does t.Fatalf when the cassette is missing, so CI without cassettes AND without AZURE_TENANT_ID hard-failed instead of skipping. Fix: check cassette existence + AZURE_TENANT_ID before recording.Start, skip cleanly if both absent. Keep the original post-Start nil-session skip as defensive guard. (The companion completion_test.go phantom-failure fix from this same commit was superseded by upstream PR #7714 during rebase, which adds a trailing newline in the renderer plus stdout capture.) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… creds Test_CLI_Up_Down_ContainerApp_RemoteBuild_MultiService called recording.Start(t) before its skip check. recording.Start does t.Fatalf when the cassette is missing, so CI without cassettes AND without AZURE_TENANT_ID hard-failed instead of skipping. Fix: check cassette existence + AZURE_TENANT_ID before recording.Start, skip cleanly if both absent. Keep the original post-Start nil-session skip as defensive guard. (The companion completion_test.go phantom-failure fix from this same commit was superseded by upstream PR #7714 during rebase, which adds a trailing newline in the renderer plus stdout capture.) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… creds Test_CLI_Up_Down_ContainerApp_RemoteBuild_MultiService called recording.Start(t) before its skip check. recording.Start does t.Fatalf when the cassette is missing, so CI without cassettes AND without AZURE_TENANT_ID hard-failed instead of skipping. Fix: check cassette existence + AZURE_TENANT_ID before recording.Start, skip cleanly if both absent. Keep the original post-Start nil-session skip as defensive guard. (The companion completion_test.go phantom-failure fix from this same commit was superseded by upstream PR #7714 during rebase, which adds a trailing newline in the renderer plus stdout capture.) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
azd completion figalways writes a trailing newline so gotestsum/JUnit does not misparse passing test output as(unknown)failuresinternal/repositorytests and only emit it when a test failsProblem
CI has been surfacing bogus Unknown failures even when the underlying test passed, for example:
The failures were caused by test output interleaving with
go test -json/gotestsum markers.Testing
go test ./... -shortFixes #7731