Skip to content

Re-enable Docker Compose CLI E2E deployment tests - #19420

Closed
Mitch Denny (mitchdenny) wants to merge 1 commit into
mainfrom
mitchdenny-re-enable-docker-compose-e2e-tests
Closed

Re-enable Docker Compose CLI E2E deployment tests#19420
Mitch Denny (mitchdenny) wants to merge 1 commit into
mainfrom
mitchdenny-re-enable-docker-compose-e2e-tests

Conversation

@mitchdenny

@mitchdenny Mitch Denny (mitchdenny) commented Aug 16, 2026

Copy link
Copy Markdown
Member

Description

Re-enables DockerDeploymentTests.CreateAndDeployToDockerCompose and DockerDeploymentTests.CreateAndDeployToDockerComposeInteractive, which were disabled twice over ([ActiveIssue] and [QuarantinedTest]) and therefore ran in no workflow — not even the quarantine one.

Root cause, per attribute

Neither disabling reason still applies.

1. [QuarantinedTest] — originally #15511, via #15515. Not flakiness. On release branches StabilizePackageVersion=true created a version split: stable packages resolved as 13.2.0 / assembly 13.2.0.0, while SuppressFinalPackageVersion=true packages such as Aspire.Hosting.Docker resolved as 13.2.0-ci / assembly 42.42.42.42, producing a CS1705 assembly version mismatch. #15883 later relabelled the quarantine URLs to the per-test failing-test issues #15882 / #15871.

2. [ActiveIssue("#15930")] — via #15931. At the time, both tests installed the CLI behind an if (isCI) guard with no else branch, so outside a PR context nothing was installed and they failed at aspire new with bash: aspire: command not found. That is exactly the captured signature in #15882 and #15871 — so those two issues are the same defect as #15930, not distinct Docker bugs.

Worth noting: issue #15930's body states "Docker-based tests don't have this problem", yet lists these two as affected. That is not a contradiction — at the time of #15931 these tests called CliE2ETestHelpers.CreateTestTerminal(), a bare (non-Docker) terminal. They only moved to CreateDockerTestTerminal() later, in the harness unification. The ActiveIssue was correct when applied and is stale now.

Why they pass now

The harness work in #16131, #16298 and #16454 replaced the ad-hoc install block with CliInstallStrategy.Detect() + InstallAspireCliAsync(). Detect() now ends in explicit fallbacks:

if (!string.IsNullOrEmpty(Environment.GetEnvironmentVariable("CI")) ||
    !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("GITHUB_ACTIONS")))
{
    return FromQuality(CliInstallQuality.Dev);   // CI fallback
}
return LatestGa();                               // local fallback

That is precisely the missing else branch #15930 described. VerifyPullRequestCliVersionAsync() also no-ops when there is no PR head SHA. The fix landed months ago; nobody removed the attributes.

Why the class had no CI job

eng/scripts/split-test-projects-for-ci.ps1 enumerates per-class jobs with --filter-not-trait quarantined=true, so a class whose tests are all quarantined disappears from the matrix entirely. Removing ActiveIssue alone would not have produced a job — both attributes had to go.

Verified with CI's exact filter set:

Enumerated classes DockerDeploymentTests present
Before 81 no
After 82 yes

Confirmed in real CI on this PR: there are now 82 Cli.EndToEnd-* jobs and Tests / Cli.EndToEnd-DockerDeploymentTests (ubuntu-latest) exists and passes.

Evidence

CI (authoritative): Cli.EndToEnd-DockerDeploymentTestspass, 5m33s. The test step itself ran 2m38s, consistent with two real Compose deployments (an all-skipped class job returns in seconds under --ignore-exit-code 8). Whole-PR status: 363 pass, 0 fail.

Local (macOS arm64, Docker 29.4.3, ASPIRE_E2E_ARCHIVE localhive build): these were quarantined for suspected flakiness, so the class was run 10 times, not once:

Iterations Test executions Pass rate Per-iteration time
10 20 100% 142–156s

No flakes, no retries, no assertion changes. Attributes were removed with QuarantineTools (-u -m activeissue, then -u) per AGENTS.md, not by hand.

Fixes #15930
Fixes #15882
Fixes #15871

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
    • No

Both tests in DockerDeploymentTests were disabled twice over:

- [QuarantinedTest] was added in #15515 for issue #15511 — a release-branch
  package version split (SuppressFinalPackageVersion=true) that produced a
  CS1705 assembly version mismatch for Aspire.Hosting.Docker. That was a
  release-branch condition, not test flakiness. #15883 later relabelled the
  quarantine URLs to per-test failing-test issues #15882/#15871.

- [ActiveIssue] for #15930 was added in #15931 because the tests installed the
  CLI behind an `if (isCI)` guard with no else branch, so outside a PR context
  they failed with `bash: aspire: command not found`. At that time these tests
  used CliE2ETestHelpers.CreateTestTerminal() — a bare (non-Docker) terminal —
  which is exactly the class of test #15930 describes.

Both root causes are gone. The harness unification in #16131, #16298 and #16454
replaced the ad-hoc install block with CliInstallStrategy.Detect() +
InstallAspireCliAsync(), which has an explicit CI fallback (InstallScript,
quality=dev) and a local fallback (latest GA), and moved these tests onto
CreateDockerTestTerminal(). VerifyPullRequestCliVersionAsync() also no-ops
outside a PR context.

Because eng/scripts/split-test-projects-for-ci.ps1 enumerates CI jobs with
`--filter-not-trait quarantined=true`, a fully quarantined class produced no CI
job at all. Removing both attributes restores the per-class job: the enumerated
class list goes from 81 to 82 and now includes
Aspire.Cli.EndToEnd.Tests.DockerDeploymentTests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 16, 2026 07:20
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19420

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19420"

@github-actions

Copy link
Copy Markdown
Contributor

Tests selector (audit mode)

The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement.

1 / 101 test projects · 2 jobs, from 1 changed file.

Selected test projects (1 / 101)

Aspire.Cli.EndToEnd.Tests

Selected jobs (2)

cli-starter, extension-e2e


How these were chosen — grouped by what changed

🧪 tests/Aspire.Cli.EndToEnd.Tests/DockerDeploymentTests.cs (changed test)
1 directly: Aspire.Cli.EndToEnd.Tests

Job reasons

Job Triggered by
cli-starter selected test Aspire.Cli.EndToEnd.Tests
extension-e2e tests/Aspire.Cli.EndToEnd.Tests/DockerDeploymentTests.cs

Selection computed for commit 74d11fd.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Re-enables Docker Compose CLI deployment E2E coverage after install-harness fixes made the disabling attributes obsolete.

Changes:

  • Removes ActiveIssue and QuarantinedTest from both Docker deployment tests.
  • Removes the unused test-utilities import.
Show a summary per file
File Description
tests/Aspire.Cli.EndToEnd.Tests/DockerDeploymentTests.cs Restores both Docker Compose deployment tests to normal CI execution.

Review details

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment on lines 19 to 20
[Fact]
[ActiveIssue("https://github.com/microsoft/aspire/issues/15930")]
[QuarantinedTest("https://github.com/microsoft/aspire/issues/15882")]
public async Task CreateAndDeployToDockerCompose()
@mitchdenny
Mitch Denny (mitchdenny) marked this pull request as ready for review August 16, 2026 07:50
@mitchdenny

Copy link
Copy Markdown
Member Author

Superseded by #19404, which will carry the attribute removal alongside the Docker Compose persistent-volume E2E coverage it unblocks. Keeping both in one PR avoids landing the re-enable and its first real consumer separately.

The investigation and evidence recorded here still stand: the [QuarantinedTest] traced to the CS1705 release-branch assembly version mismatch (#15511/#15515), and the [ActiveIssue] (#15930) was correctly applied when these tests used a bare-bash terminal but went stale once the harness unification (#16131, #16298, #16454) moved them to CreateDockerTestTerminal and added the CI install fallback. Verified 10 local iterations at 100% pass plus a green Cli.EndToEnd-DockerDeploymentTests CI job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants