Skip to content

ci: run helm unittest on all charts - #966

Open
marcleblanc2 wants to merge 4 commits into
mainfrom
marc/helm-unittest-all-charts
Open

marcleblanc2 wants to merge 4 commits into
mainfrom
marc/helm-unittest-all-charts

Conversation

@marcleblanc2

@marcleblanc2 marcleblanc2 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #930 / #944, which made scripts/ci/lint.sh cover every chart; now scripts/ci/helm-unittest.sh also unit tests each chart

  • Pass all four charts to helm unittest. The plugin already runs every chart after a failure, prints a per-chart summary, and exits non-zero if any chart failed, so no bash bookkeeping is needed.
  • Drop -q. In helm-unittest that flag is --failfast, not quiet, so it stopped at the first failing test and hid the rest
  • Added a unit tests for charts/sourcegraph-migrator, covering the Job's random name suffix, default and overridden args, the three database secretKeyRefs (default names and a per-database existingSecret override), migrator.env, and priorityClassName precedence (service over global, unset by default)
  • TEST.md now shows the all-charts helm unittest command CI uses and where each chart's tests live
  • The charts/sourcegraph-executor/dind tests had never run in CI and still targeted the chart as it was before feat(dind): multi-queue support, Docker mode fixes, and gVisor sandbox support #876: executor/executor.Service.yaml, executor.enabled, and a registry Deployment + PVC, none of which exist anymore. Rewrote them for the current templates: ConfigMap + executor-<queueName> Deployment with EXECUTOR_QUEUE_NAME, one Deployment per queues[] entry, and the registry StatefulSet + Service gated on privateDockerRegistry.enabled.

Amp-Thread-ID: https://ampcode.com/threads/T-01a156fc-33c6-7408-8f03-c6410e72484a

Checklist

CI, test, and TEST.md-only change, no chart templates or values touched, so no changelog or update-doc entry.

Test plan

Before, on main, the dind chart fails on its own:

$ helm unittest charts/sourcegraph-executor/dind
 FAIL  executor	charts/sourcegraph-executor/dind/tests/executor_test.yaml
 FAIL  privateDockerRegistry	charts/sourcegraph-executor/dind/tests/privateDockerRegistry_test.yaml
Charts:      1 failed, 0 passed, 1 total
Tests:       4 failed, 0 passed, 4 total

After, the script run with the plugin already installed (helm-unittest v1.0.2, helm 4.3.0):

### Chart [ sourcegraph ] charts/sourcegraph
### Chart [ sourcegraph-migrator ] charts/sourcegraph-migrator
### Chart [ sourcegraph-executor-k8s ] charts/sourcegraph-executor/k8s
### Chart [ sourcegraph-executor-dind ] charts/sourcegraph-executor/dind
Charts:      4 passed, 4 total
Test Suites: 36 passed, 36 total
Tests:       214 passed, 214 total

Negative checks: breaking the migrator template's codeintel secret wiring and priorityClassName precedence fails 3 of the 8 new migrator tests. Changing one dind assertion to a wrong Deployment name makes the script exit 1 while the other three charts still run and report.

bash -n scripts/ci/helm-unittest.sh passes.

- scripts/ci/helm-unittest.sh only tested charts/sourcegraph; pass all four charts so the executor and migrator charts are tested too
- drop -q, which is --failfast in helm-unittest and hid every failure after the first
- the dind executor tests still targeted the pre-#876 chart (executor.Service.yaml, executor.enabled, registry Deployment + PVC) and had never run in CI; rewrite them for the current templates

Amp-Thread-ID: https://ampcode.com/threads/T-01a156fc-33c6-7408-8f03-c6410e72484a
Co-authored-by: Amp <amp@ampcode.com>
@github-actions

Copy link
Copy Markdown

- cover the Job name suffix, default and overridden args, database secret wiring (default and existingSecret), migrator.env, and priorityClassName precedence
- TEST.md: show the all-charts helm unittest command CI uses and where each chart's tests live

Amp-Thread-ID: https://ampcode.com/threads/T-01a156fc-33c6-7408-8f03-c6410e72484a
Co-authored-by: Amp <amp@ampcode.com>
Comment thread charts/sourcegraph-migrator/tests/migrator_test.yaml Fixed
marcleblanc2 and others added 2 commits October 19, 2026 20:02
Prose is reflowed and the three long shell commands use line continuations; no wording changes.

Amp-Thread-ID: https://ampcode.com/threads/T-01a156fc-33c6-7408-8f03-c6410e72484a
Co-authored-by: Amp <amp@ampcode.com>
…chart

Checkov's secrets scanner (CKV_SECRET_6) flagged my-pgsql-secret as a high-entropy string; my-secret matches the sourcegraph chart's tests and is not flagged.

Amp-Thread-ID: https://ampcode.com/threads/T-01a156fc-33c6-7408-8f03-c6410e72484a
Co-authored-by: Amp <amp@ampcode.com>
@marcleblanc2 marcleblanc2 changed the title ci: run helm unittest against all charts ci: run helm unittest on all charts Oct 20, 2026
@marcleblanc2
marcleblanc2 requested a review from a team October 20, 2026 07:08
@marcleblanc2
marcleblanc2 enabled auto-merge (squash) October 20, 2026 07:08

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants