Improve code coverage: Phase 1-2 tests, coverage tooling, and developer workflow#7424
Merged
Improve code coverage: Phase 1-2 tests, coverage tooling, and developer workflow#7424
Conversation
jongio
requested review from
JeffreyCA,
danieljurek,
hemarina,
rajeshkamal5050,
tg-msft,
vhvb1989,
wbreza and
weikanglim
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves overall azd code coverage by adding substantial unit-test coverage across multiple packages, introducing developer-focused local/CI coverage tooling (including generated-code filtering), documenting the workflow, and raising the CI minimum coverage gate.
Changes:
- Adds extensive new unit tests across
pkg/*,internal/*, andcmd/*to raise coverage. - Introduces local coverage tooling (
Get-LocalCoverageReport.ps1) and generated-code filtering (Filter-GeneratedCoverage.ps1), and wires filtering into CI coverage merge. - Documents the coverage workflow and raises the CI minimum coverage threshold from 52% to 55%.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| eng/scripts/Test-CodeCoverageThreshold.ps1 | Expands script help text and references the new coverage guide. |
| eng/scripts/Get-LocalCoverageReport.ps1 | Adds a local coverage workflow script supporting unit-only, hybrid CI-merge, and full combined modes. |
| eng/scripts/Get-CICoverageReport.ps1 | Updates guidance and filters generated code from merged CI coverage profiles. |
| eng/scripts/Filter-GeneratedCoverage.ps1 | New script to remove generated files (e.g., *.pb.go) from Go coverage profiles. |
| eng/pipelines/templates/stages/code-coverage-upload.yml | Adds generated-code filtering step before Cobertura conversion in CI. |
| eng/pipelines/release-cli.yml | Raises MinimumCoveragePercent coverage gate to 55%. |
| cli/azd/docs/code-coverage-guide.md | New deep-dive documentation for coverage architecture and usage modes. |
| cli/azd/CONTRIBUTING.md | Adds a “Code Coverage” section describing recommended workflows and commands. |
| cli/azd/pkg/tools/github/github_methods_test.go | New unit tests for GitHub CLI wrapper behaviors and error handling. |
| cli/azd/pkg/sqldb/sqldb_test.go | New tests for SQL DB connection string generation and error paths. |
| cli/azd/pkg/cosmosdb/cosmosdb_test.go | New tests for Cosmos DB connection string retrieval and edge cases. |
| cli/azd/pkg/armmsi/armmsi_test.go | New tests for ARM MSI service methods and response handling. |
| cli/azd/pkg/keyvault/keyvault_coverage_test.go | Expanded coverage for Key Vault parsing/validation and secret resolution helpers. |
| cli/azd/pkg/auth/wave3_coverage_test.go | Large suite of auth manager tests covering additional branches and migrations. |
| cli/azd/pkg/auth/remote_credential_coverage_test.go | Adds unit tests for RemoteCredential transport/JSON/error scenarios. |
| cli/azd/pkg/auth/final_coverage_test.go | Adds targeted tests for remaining auth/cache edge branches. |
| cli/azd/pkg/auth/credential_providers_coverage_test.go | Adds coverage for multi-tenant credential provider caching and error paths. |
| cli/azd/pkg/auth/cache_coverage_test.go | Adds tests for cache implementations (memory/file) and adapter behavior. |
| cli/azd/pkg/auth/additional_coverage_test.go | Adds additional credential/auth/login/logout branch coverage. |
| cli/azd/internal/mcp/proxy_handlers_test.go | Adds a small test for MCP proxy initialization success path. |
| cli/azd/internal/mcp/host_test.go | Adds broad tests for MCP host start/stop and tool proxying behaviors. |
| cli/azd/cmd/vs_server_test.go | Adds a test for certificate generation logic used by VS server. |
| cli/azd/cmd/version_test.go | Adds tests for version command formatting and channel suffix logic. |
| cli/azd/cmd/util_test.go | Adds tests for utility helpers (parsing, warnings, overrides). |
| cli/azd/cmd/update_test.go | Adds tests for update command flag persistence logic. |
| cli/azd/cmd/templates_test.go | Adds tests for template command help/flags and constructors. |
| cli/azd/cmd/misc_test.go | Adds tests for assorted help builders and small helper types. |
| cli/azd/cmd/infra_generate_test.go | Adds tests for duplicate-file detection helper. |
| cli/azd/cmd/extension_test.go | Adds tests around extension resolution, compatibility, and output formatting. |
| cli/azd/cmd/env_test.go | Adds tests for env command help text and parsing helpers. |
| cli/azd/cmd/copilot_test.go | Adds tests for consent description formatting helpers. |
| cli/azd/cmd/config_test.go | Adds tests for config commands (show/get/set/unset/reset) behaviors. |
| cli/azd/cmd/completion_test.go | Adds tests for completion actions, shell support, and help text. |
| cli/azd/cmd/cmd_help_test.go | Adds tests for the help generation utilities and formatting. |
| cli/azd/cmd/actions_test.go | Adds tests for small flag pointer helper types and upload action behavior. |
| cli/azd/cmd/auth_login_test.go | Adds tests for Codespaces browser detection and device-code parsing. |
Member
Author
|
@copilot please re-review the latest changes. |
Contributor
Re-reviewed and fixed the two CI failures introduced in the latest commits:
All three previous review threads (stderr redirect, gofmt, keyvault sentinel test) remain fixed. |
wbreza
approved these changes
Apr 2, 2026
Contributor
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Code Review — Coverage Improvement Phase 1-2
Well-structured PR with zero backwards-compatibility risk — all 12K+ lines are purely additive (new tests, mage targets, scripts, docs). No existing source code or APIs are modified.
What Looks Good ✅
- Clean test patterns — table-driven tests, proper testify usage,
t.Parallel(),t.TempDir(), Go 1.26t.Context()andnew(val)patterns used consistently across all new test files - Well-documented magefile.go —
Coveragenamespace with properGOWORK=offCI mirroring and cross-platform PowerShell discovery - CI gate increase is safe — 52%→55% threshold uses the PR's own pipeline YAML, so in-flight PRs based on old main are unaffected
- Good generated-code filtering —
Filter-GeneratedCoverage.ps1is extensible, handles edge cases, and integrates cleanly into CI - Proper copyright headers on all new files
- No global state pollution — tests use
t.TempDir()and mocks, no unscopedos.Setenvoros.Chdir
Findings
| Priority | Count |
|---|---|
| Low | 1 |
| Total | 1 |
[Low] mage coverage:check defaults to 50% while CI gates at 55%. The doc comment explains the reasoning well (unit-only vs combined), but consider mentioning COVERAGE_MIN=55 mage coverage:check in CONTRIBUTING.md so contributors can replicate the CI gate locally.
Backwards Compatibility: ✅ Safe
All changes are additive — no existing code, tests, APIs, or CI behavior is modified. The only "behavioral" change is the CI gate increase (52→55%), which is gated by this PR's own test additions.
Overall: ✅ Approve
…developer workflow - Add 16 new test files for cmd/, middleware/, mcp/, and github packages - Create local coverage orchestrator (Get-LocalCoverageReport.ps1) with -UnitOnly, -MergeWithCI hybrid mode, -Html report options - Add generated code filter (Filter-GeneratedCoverage.ps1) for *.pb.go - Integrate filter into CI pipeline (code-coverage-upload.yml) - Raise CI coverage gate from 52% to 55% (release-cli.yml) - Add Code Coverage section to CONTRIBUTING.md with 4-mode matrix - Create cli/azd/docs/code-coverage-guide.md deep-dive reference - Cross-reference all 4 coverage scripts to each other and guide Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…qldb, armmsi - pkg/auth: 7 new test files, 54.2% -> 80.0% (+25.8pp) Covers CredentialForCurrentUser branches, loadClaims/saveClaims, Mode/SetBuiltInAuthMode, Logout cleanup, RemoteCredential, caches - pkg/keyvault: ParseKeyVaultAppReference edge cases, validation, 37.8% -> 41.5% - pkg/pipeline: Template rendering, provider lifecycle, pure logic, 27.8% -> 30.1% - pkg/cosmosdb: New test file from 0% -> 86.7% (mock ARM client) - pkg/sqldb: New test file from 0% -> 94.4% (mock ARM client) - pkg/armmsi: New test file from 0% -> 76.7% (mock ARM client) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add missing .PARAMETER PipelineDefinitionId to Get-CICoverageReport.ps1 help - Add working directory note to CONTRIBUTING.md Code Coverage section - Add Scripts Reference table to code-coverage-guide.md listing all 7 scripts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Run gofmt -s on all new test files - Remove unused fakeHTTPClient from credential_providers_coverage_test.go - Remove unused jwtTokenCredential from wave3_coverage_test.go - Break long line in middleware_coverage2_test.go (lll) - Fix trailing whitespace in pipeline_coverage_test.go Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add Coverage namespace to magefile with 6 targets (unit, full, hybrid, ci, html, check) - Update CONTRIBUTING.md and code-coverage-guide.md with mage target docs - Fix stderr redirect in Get-LocalCoverageReport.ps1 token acquisition (review thread #1) - Fix tautological sentinel test in keyvault_coverage_test.go (review thread #3) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…versal Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/75a62821-a5d2-4f5b-8372-b3b8fe8d1802 Co-authored-by: jongio <2163001+jongio@users.noreply.github.com>
- middleware_coverage2_test: skip TestLoginGuard_EnsureLogin_ConfirmError in CI where IsRunningOnCI() short-circuits before console Confirm - manager_coverage_test: use t.Setenv+os.Unsetenv for proper env var isolation (code uses os.LookupEnv which distinguishes empty vs unset) - Remove bare os.Unsetenv calls that mutated shared process state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The coverage:check mage target uses unit-only mode (52.7%) but was defaulting to 55% threshold (designed for CI combined unit+integration). Lower to 50% which is appropriate for unit-only, and update docs to clarify the distinction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Rebase onto latest main to pick up error.go refactor (PR #7401) - Replace classifyError/ErrorCategory tests with fixableError tests - Fix ExtensionRunError struct fields (ExitCode -> ExtensionId+Err) - Add COVERAGE_MIN override tip to CONTRIBUTING.md (reviewer feedback) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
force-pushed
the
jongio/coverage-improvement-phase1-2
branch
from
aeb6248 to
f7a2b79
Compare
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
|
wbreza
approved these changes
Apr 2, 2026
Contributor
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Code Review — Prior Feedback Verification
All 4 prior review threads have been verified as resolved in the current diff:
| # | File | Prior Issue | Status |
|---|---|---|---|
| 1 | eng/scripts/Get-LocalCoverageReport.ps1 |
stderr redirect could corrupt ADO token | ✅ Fixed — redirect removed, proper error checking added |
| 2 | cli/azd/cmd/auth_login_test.go |
Not gofmt'd | ✅ Fixed in commit 8a194e2 |
| 3 | cli/azd/pkg/keyvault/keyvault_coverage_test.go |
Tautological sentinel test | ✅ Fixed — wraps sentinel, tests unwrapping chain, asserts negative case |
| 4 | cli/azd/magefile.go |
Document COVERAGE_MIN=55 | ✅ Fixed — added to CONTRIBUTING.md and code-coverage-guide.md |
✅ What Looks Good
- All prior feedback addressed thoroughly with proper fixes
- Purely additive PR — zero backwards-compatibility risk
- Consistent Go 1.26 test patterns throughout
No new issues found. All prior feedback resolved.
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.
Summary
Raises unit test coverage and introduces a 4-mode developer coverage workflow for the azd CLI. This PR delivers Phase 1-2 of a planned coverage improvement initiative, adding ~12K lines of new tests and tooling.
CI coverage gate: 52% → 55% (new minimum threshold)
What's Changed
New Tests (Phase 1-2)
Coverage Tooling
eng/scripts/Get-LocalCoverageReport.ps1— Single-script developer coverage tool with 4 modes:eng/scripts/Filter-GeneratedCoverage.ps1— Filters protobuf.pb.gofiles from coverage datamage coverage:unit,coverage:full,coverage:mergeWithCI,coverage:ciBaseline,coverage:html,coverage:checkCI Pipeline
eng/pipelines/release-cli.yml— MinimumCoveragePercent: 52 → 55eng/pipelines/templates/stages/code-coverage-upload.yml— Generated code filteringeng/scripts/Get-CICoverageReport.ps1— Enhanced with filter support, PipelineDefinitionIdeng/scripts/Test-CodeCoverageThreshold.ps1— Cross-reference supportDocumentation
cli/azd/CONTRIBUTING.md— New "Code Coverage" section with 4-mode table and Mage targetscli/azd/docs/code-coverage-guide.md— Deep-dive reference: architecture, modes, prerequisites, troubleshootingTesting
mage preflightpasses (gofmt, go fix, copyright, golangci-lint, cspell, build, tests)Related
Phase 3-4 (interface injection for Azure-dependent code, fuzz testing) deferred to follow-up PRs.