chore: add deployment version e2e tests#903
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
📝 WalkthroughWalkthroughAdds a new Go test suite Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a new end-to-end style controller pipeline test suite to validate deployment version behavior (selection, rollout gating, and job dispatch) in workspace-engine.
Changes:
- Introduces
version_test.gocovering version selection (paused/rejected/building/failed), ordering, and metadata-based filtering. - Adds re-evaluation scenarios (new version added, active version rejected) and rollout gating scenarios (approval gate, cooldown, gradual rollout).
- Validates job dispatch behavior across multiple resources and agents, plus job-agent config propagation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| p.AssertReleaseVersion(t, 0, "v3.0.0") | ||
| p.AssertJobCreated(t) | ||
|
|
||
| release := p.Releases()[0] |
There was a problem hiding this comment.
TestVersion_ReleaseLinkedToCorrectVersion asserts job.ReleaseId == release.Id.String(), but in the current test harness the desired-release controller/mock setter does not populate oapi.Release.Id (jobs are created with ReleaseId: r.Id.String()), so both values are typically the zero UUID. This makes the assertion effectively vacuous and won’t catch broken job↔release linkage. Suggestion: update the harness to assign a stable non-zero release ID (e.g., r.Id = r.UUID() or equivalent) and set job.ReleaseId accordingly, or change the test to assert against that stable ID so the linkage is actually verified.
| release := p.Releases()[0] | |
| release := p.Releases()[0] | |
| require.NotEqual(t, uuid.Nil, release.Id, "release ID must be non-zero to validate job linkage") |
| p.ReleaseGetter.Releases = map[string]*oapi.Release{ | ||
| release.Id.String(): release, | ||
| } | ||
| p.ReleaseGetter.JobVerificationStatuses = map[string]oapi.JobVerificationStatus{ | ||
| job.Id: oapi.JobVerificationStatusPassed, | ||
| } |
There was a problem hiding this comment.
This test keys ReleaseGetter.Releases by release.Id.String(). In the current harness, releases typically have an unset/zero Id, so this map will use the same key for every release and can hide issues when multiple releases are involved. Prefer keying by a stable non-zero release ID (and ensure the harness sets it, e.g., via deterministic Release.UUID()) so GetReleaseByJobID/cooldown evaluation can’t accidentally resolve the wrong release.
| func TestVersion_ConfigAndMetadataFlowToRelease(t *testing.T) { | ||
| p := NewTestPipeline(t, | ||
| WithDeployment(DeploymentSelector("true")), | ||
| WithEnvironment(EnvironmentName("production")), | ||
| WithResource(ResourceName("srv"), ResourceKind("Server")), | ||
| WithVersion( | ||
| VersionTag("v1.0.0"), | ||
| VersionMetadata(map[string]string{ | ||
| "git-sha": "abc123", | ||
| "build-url": "https://ci.example.com/builds/42", | ||
| "image-repo": "ghcr.io/myorg/myapp", | ||
| }), | ||
| ), | ||
| ) | ||
|
|
||
| p.Run() | ||
|
|
||
| p.AssertReleaseCreated(t) | ||
| release := p.Releases()[0] | ||
| assert.Equal(t, "v1.0.0", release.Version.Tag) | ||
| assert.Equal(t, "abc123", release.Version.Metadata["git-sha"]) | ||
| assert.Equal(t, "https://ci.example.com/builds/42", release.Version.Metadata["build-url"]) | ||
| assert.Equal(t, "ghcr.io/myorg/myapp", release.Version.Metadata["image-repo"]) | ||
| } |
There was a problem hiding this comment.
Test name says “ConfigAndMetadataFlowToRelease”, but the setup/assertions only cover VersionMetadata and never set/assert DeploymentVersion.Config. Either rename the test to reflect metadata-only behavior, or add a VersionOption that sets v.Config and assert the config is present on release.Version.Config as well.
| func TestVersion_VersionConfigFlowsToRelease(t *testing.T) { | ||
| p := NewTestPipeline(t, | ||
| WithDeployment(DeploymentSelector("true")), | ||
| WithEnvironment(EnvironmentName("production")), | ||
| WithResource(ResourceName("srv"), ResourceKind("Server")), | ||
| WithVersion( | ||
| VersionTag("v1.0.0"), | ||
| VersionMetadata(map[string]string{"deploy-strategy": "blue-green"}), | ||
| ), | ||
| WithJobAgent("deployer"), | ||
| ) | ||
|
|
||
| p.Run() | ||
|
|
||
| p.AssertReleaseCreated(t) | ||
| p.AssertJobCreated(t) | ||
|
|
||
| release := p.Releases()[0] | ||
| assert.Equal(t, "blue-green", release.Version.Metadata["deploy-strategy"]) | ||
| p.AssertJobReleaseID(t, 0, release.Id.String()) | ||
| } |
There was a problem hiding this comment.
TestVersion_VersionConfigFlowsToRelease only sets/asserts metadata (VersionMetadata / release.Version.Metadata) and then asserts job↔release linkage via release.Id, which is typically unset/zero in the harness. Consider renaming this test to “...Metadata...” (or actually set/assert DeploymentVersion.Config), and avoid asserting linkage via release.Id until the harness assigns stable non-zero IDs (e.g., release.UUID()).
Summary by CodeRabbit