Fix TestDetect flakiness from concurrent Maven access to shared .m2#8837
Merged
Conversation
… subtests Co-authored-by: JeffreyCA <9157833+JeffreyCA@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix TestDetect flakiness from concurrent Maven access to shared .m2
Fix TestDetect flakiness from concurrent Maven access to shared .m2
Jun 26, 2026
Contributor
|
/azp run azure-dev - cli |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces test flakiness in appdetect by preventing concurrent Maven invocations from running as parallel subtests and racing on a cold ~/.m2 cache (notably on Windows CI).
Changes:
- Removes
t.Parallel()from theTestDetectinner subtest loop so Maven-touching cases run sequentially. - Preserves
t.Parallel()at theTestDetectfunction level so the package can still run concurrently with other tests.
Show a summary per file
| File | Description |
|---|---|
cli/azd/internal/appdetect/appdetect_test.go |
Serializes TestDetect subtests to avoid concurrent Maven access to the shared ~/.m2 cache. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Low
jongio
reviewed
Jun 29, 2026
jongio
left a comment
jongio
left a comment
Member
There was a problem hiding this comment.
Confirmed the root cause: Full and ExcludePatterns subtests both invoke mvn help:effective-pom, racing on the shared ~/.m2 cache during cold starts. Serializing inner subtests while preserving the outer t.Parallel() is the right trade-off: 4 subtests running sequentially is negligible cost, and it prevents someone from accidentally reintroducing the race by adding another Java-touching subtest.
One minor suggestion inline.
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
jongio
reviewed
Jun 29, 2026
jongio
left a comment
jongio
left a comment
Member
There was a problem hiding this comment.
Incremental review (1 new commit since my last pass):
The added comment correctly documents why inner subtests don't call t.Parallel(). Placement before the for loop is appropriate since it applies to all subtests in the table. The wording explains both the constraint (concurrent Maven invocations racing on ~/.m2) and the consequence (flakiness), which should prevent someone from re-adding parallelism later.
No new issues. Prior feedback addressed.
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
|
vhvb1989
approved these changes
Jun 29, 2026
JeffreyCA
approved these changes
Jun 29, 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
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.
TestDetect/FullandTestDetect/ExcludePatternsboth call realmvn help:effective-pomand ran as parallel subtests, racing to download artifacts into the shared~/.m2cache on a cold start — causing intermittent "Could not transfer artifact ... .tmp" failures on Windows CI.Changes
internal/appdetect/appdetect_test.go: Removet.Parallel()from the inner subtest loop inTestDetect, serializing the Maven-touching subtests relative to each other.The outer
t.Parallel()onTestDetectitself is preserved — the function still runs in parallel with other packages, only the subtests within it are now sequential.