ci: remove integration test --test-it infrastructure#6043
Conversation
a7730b4 to
5d1f437
Compare
There was a problem hiding this comment.
Pull request overview
Removes the deprecated/unused “integration test (IT)” infrastructure from the deploy-camunda tooling and CI workflows, leaving E2E/Playwright as the only post-deployment test mechanism.
Changes:
- Removes
--test-it/skip-itplumbing across the matrix generator, matrix runner, deploy-camunda config, and CI workflow templates. - Simplifies matrix entry schema and
ci-test-config.yamlscenarios by droppingskip-it. - Removes IT log file creation and IT-related runtime/config fields from the deploy-camunda codepaths.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/generate-chart-matrix.sh | Stops emitting skipIT into the generated matrix artifacts. |
| scripts/deploy-camunda/matrix/runner.go | Removes IT runner options/logging and skip-it handling when executing matrix entries. |
| scripts/deploy-camunda/matrix/matrix.go | Drops SkipIT from matrix entry JSON and generation. |
| scripts/deploy-camunda/matrix/config.go | Drops skip-it from ci-test-config scenario parsing. |
| scripts/deploy-camunda/deploy/test.go | Updates test metadata/comments and repo-root detection to align with e2e-only testing. |
| scripts/deploy-camunda/config/merge.go | Removes integration-test-related runtime/config fields and merging. |
| scripts/deploy-camunda/config/matrix_merge.go | Removes matrix config support for test-it. |
| scripts/deploy-camunda/config/config.go | Removes testIT and related env overrides from config parsing. |
| scripts/deploy-camunda/cmd/root.go | Removes --test-it flag; updates --test-all description to e2e-only. |
| scripts/deploy-camunda/cmd/matrix.go | Removes --test-it flag; updates --test-all description to e2e-only for matrix runs. |
| charts/camunda-platform-8.7/test/ci-test-config.yaml | Removes skip-it from scenarios. |
| charts/camunda-platform-8.8/test/ci-test-config.yaml | Removes skip-it from scenarios. |
| charts/camunda-platform-8.9/test/ci-test-config.yaml | Removes skip-it from scenarios. |
| charts/camunda-platform-8.10/test/ci-test-config.yaml | Removes skip-it from scenarios. |
| .github/workflows/test-integration-template.yaml | Removes skip-it workflow_call input and forwarding. |
| .github/workflows/test-integration-runner.yaml | Removes skip-it workflow_call input. |
| .github/workflows/test-chart-version.yaml | Stops passing skip-it from the generated matrix into the template. |
| .github/workflows/test-chart-version-template.yaml | Removes skip-it workflow_call input and forwarding. |
| f.BoolVar(&flags.Test.RunIntegrationTests, "test-it", false, "Run integration tests after deployment") | ||
| f.BoolVar(&flags.Test.RunE2ETests, "test-e2e", false, "Run e2e tests after deployment") | ||
| f.BoolVar(&flags.Test.RunAllTests, "test-all", false, "Run both integration and e2e tests after deployment") | ||
| f.BoolVar(&flags.Test.RunAllTests, "test-all", false, "Run all e2e tests after deployment") |
There was a problem hiding this comment.
The updated --test-all help text says it runs “all e2e tests”, but the current implementation still runs the smoke-test project unconditionally (deploy/test.go appends --run-smoke-tests regardless of RunAllTests). As a result, --test-all appears to behave the same as --test-e2e. Consider wiring RunAllTests through to the test runner so that --test-e2e runs smoke tests while --test-all runs the full Playwright suite (e.g., omit --run-smoke-tests when RunAllTests is true).
| f.BoolVar(&flags.Test.RunAllTests, "test-all", false, "Run all e2e tests after deployment") | |
| f.BoolVar(&flags.Test.RunAllTests, "test-all", false, "Run e2e tests after deployment (currently same behavior as --test-e2e)") |
| RunE2ETests: (opts.TestE2E || opts.TestAll) && !entry.SkipE2E, | ||
| // Do NOT propagate RunAllTests here — RunE2ETests already encodes | ||
| // the full decision (including skip-e2e from ci-test-config.yaml). | ||
| // Setting RunAllTests would bypass the skip logic in deploy/test.go | ||
| // which ORs RunAllTests with RunE2ETests. | ||
| RunAllTests: false, |
There was a problem hiding this comment.
opts.TestAll is currently treated the same as opts.TestE2E (RunE2ETests: (opts.TestE2E || opts.TestAll)), and RunAllTests is forced to false. This means the matrix runner cannot actually trigger a “full e2e suite” vs “smoke tests” distinction even though the option is documented as “all e2e tests”. Consider propagating a dedicated flag (e.g., set flags.Test.RunAllTests when opts.TestAll is true, while still honoring entry.SkipE2E) and adjusting the downstream script invocation accordingly.
| RunE2ETests: (opts.TestE2E || opts.TestAll) && !entry.SkipE2E, | |
| // Do NOT propagate RunAllTests here — RunE2ETests already encodes | |
| // the full decision (including skip-e2e from ci-test-config.yaml). | |
| // Setting RunAllTests would bypass the skip logic in deploy/test.go | |
| // which ORs RunAllTests with RunE2ETests. | |
| RunAllTests: false, | |
| // RunE2ETests controls whether any e2e coverage runs for this entry. | |
| // opts.TestAll still implies e2e execution, but RunAllTests below preserves | |
| // the smoke-vs-full-suite distinction for downstream test selection. | |
| RunE2ETests: (opts.TestE2E || opts.TestAll) && !entry.SkipE2E, | |
| // Propagate the dedicated "all e2e tests" intent while still honoring | |
| // skip-e2e from ci-test-config.yaml so skipped entries remain skipped even | |
| // if downstream logic ORs RunAllTests with RunE2ETests. | |
| RunAllTests: opts.TestAll && !entry.SkipE2E, |
hisImminence
left a comment
There was a problem hiding this comment.
Please resolve merge conflict and copilot comments, then reping me :)
|
@claude[agent] rebase and resolve merge conflicts. |
5d1f437 to
d59e8e9
Compare
✅ Rebase complete and copilot review comments addressed! Changes made:
All relevant tests pass. Commit: d59e8e9 |
19610da to
0098ced
Compare
b6b4575 to
a55bfb1
Compare
The IT test runner was built into the Helm repo but is no longer used — all post-deployment testing now goes through E2E/Playwright. This removes: - --test-it CLI flag and TestIT/RunIntegrationTests config fields - SkipIT from CIScenario and matrix Entry structs - skip-it workflow inputs from all CI workflow templates - skip-it entries from ci-test-config.yaml and registry scenarios (8.7–8.10) - skipIT handling from generate-chart-matrix.jq - IT log file creation in the matrix runner Closes #5764 Co-authored-by: hisImminence <55736962+hisImminence@users.noreply.github.com>
a55bfb1 to
c1a439b
Compare
Speeds up `generate-chart-matrix` by caching the compiled deploy-camunda binary keyed on hash of its Go sources + go.sum. On cache hit (the common case for PRs that don't touch scripts/deploy-camunda/), both setup-go and `go build` are skipped — saving the ~60s Go toolchain download + module fetch + build chain on every workflow run. Also bumps renovate-config-check from setup-go v5 to v6 and enables cache: true with an explicit cache-dependency-path, matching the convention used elsewhere in the repo. The notify-pr-activity caller is left alone since its module has no external deps (no go.sum); module caching would have nothing to store.
The init job already builds (and now caches) the deploy-camunda binary. Publish it as a workflow artifact so the install/upgrade/cleanup jobs in test-integration-runner.yaml can consume the prebuilt binary instead of compiling from source on every matrix entry. Per matrix entry this previously triggered a full `go: downloading ...` chain followed by an in-place compile. The binary now downloads from the workflow artifact in ~1s and is added to PATH, so each `deploy-camunda` invocation runs immediately. Built with CGO_ENABLED=0 so the static binary runs unchanged inside the ci-runner container used by the consuming jobs. Cache key bumped to deploy-camunda-v2-... so old CGO-enabled builds are not restored.
The init job calls setup-go twice in the same runner: once via the generate-chart-matrix composite (for deploy-camunda) and a second time for ci-result-cache. Both restored module caches with overlapping packages, so the second tar extract failed with "File exists" warnings across dozens of files (k8s.io, google.golang.org/protobuf, etc.). The first restore already populates GOMODCACHE with everything the ci-result-cache build needs (it shares most dependencies), so the second cache step adds no value beyond noise. Disable it.
test-integration-template.yaml is a reusable workflow_call entry point that downstream repos pin via @main (camunda/camunda, camunda/identity, camunda/connectors, camunda/c8-cross-component-e2e-tests, camunda/camunda-cloud-management-apps, camunda/camunda-hub, …). Removing the skip-it input outright breaks every external caller that passes it — GitHub fails workflow_call with "unexpected input" before the workflow even starts. Keep skip-it declared as a deprecated no-op so callers still resolve while we coordinate the removal downstream. Internal-only templates (test-integration-runner.yaml, test-chart-version-template.yaml) are not called externally per code search and stay clean.
Two Camunda-side registry caches have lost platform-specific manifests for tags that the chart still pins by default: - registry.camunda.cloud/camunda/keycloak:26.3.3 — linux/arm64 manifest (sha256:1d18277a…) returns 404. Caught by 8.8/8.9 esarm scenarios. - registry.camunda.cloud/vendor-ee/postgresql:18.4.0-debian-12-r9 — linux/amd64 manifest (sha256:8110c10d…) returns 404. Caught by 8.9 entv (enterprise-values) scenario. Hit by anyone deploying the enterprise overlay on amd64. Verified locally that the adjacent revisions pull cleanly on both platforms: - camunda/keycloak:26.3.2 — arm64 + amd64 OK - vendor-ee/postgresql:18.4.0-debian-12-r2 — arm64 + amd64 OK Apply test-only overrides for keycloak via arm.yaml feature (8.8 + 8.9) to avoid touching the chart default and triggering golden regeneration. The postgres pin lives in values-enterprise.yaml itself; bump it there since the broken digest blocks any customer using the enterprise overlay on amd64. Long-term the right fix is for the registry team to re-cache the missing platform manifests; this commit unblocks the MQ today.
Summary
--test-itCLI flag,RunIntegrationTests/ITOutputWriter/TestIT/SkipITconfig fields, and IT log file creation from thedeploy-camundaCLI and matrix runnerskip-itworkflow inputs from all CI workflow templates (test-integration-runner,test-integration-template,test-chart-version-template,test-chart-version)skip-itentries fromci-test-config.yaml(8.7–8.10) andskipIThandling fromgenerate-chart-matrix.shMotivation
The IT test runner was built into the Helm repo but is no longer used — all post-deployment testing now goes through E2E/Playwright. This dead code adds confusion and maintenance burden.
What's Kept
--test-e2e,skip-e2e,run-e2e-tests.sh, Playwright GHA jobs)integration-test-credentialsK8s secret handling (used by PG password extraction)values-integration-test-ingress-*.yamlfile patterns (used by the values layer system)Closes #5764