Skip to content

test: remove stale provider-specific Helm assertions#40

Merged
dispatch-developer[bot] merged 1 commit into
mainfrom
clean-generic-helm-fixture
Jul 20, 2026
Merged

test: remove stale provider-specific Helm assertions#40
dispatch-developer[bot] merged 1 commit into
mainfrom
clean-generic-helm-fixture

Conversation

@ajac-zero

Copy link
Copy Markdown
Owner

Summary

  • remove stale GitHub broker IDs, ports, and probe assertions from the provider-neutral Helm sidecar fixture
  • assert only the neutral provider env, volume, image, and security context that the fixture declares

Verification

  • Helm lint and full static rendering script
  • no GitHub-specific broker names, ports, or repository IDs remain in the generic fixture
  • diff whitespace validation

@dispatch-reviewer dispatch-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review: clean-generic-helm-fixture

CI passed (all steps green: typecheck, typecheck:test, unit tests, audit, dependency-review).

What changed

deploy/helm/agentbay/tests/static-render.sh — the block of assertions that immediately followed the grep -q 'name: PROVIDER_TENANT' line was removed (8 lines) and replaced with a single new assertion (1 line):

Removed (provider-specific / stale):

grep -q 'value: "123456789"'           # hard-coded GitHub tenant ID
test'exec:' … -eq 3                 # node process probes
test'- /nodejs/bin/node' … -eq 3
test'http://127.0.0.1:8083/readyz' … -eq 2
test'http://127.0.0.1:8083/livez' … -eq 1
if grep -q 'httpGet:'; then exit 1; fi

Added (matches the fixture's values):

grep -q 'value: default'               # matches PROVIDER_TENANT: default in the values block

Correctness

The fixture's connection-sidecar-values.yaml block that is rendered immediately before these assertions defines:

env:
  - name: PROVIDER_TENANT
    value: default

The new assertion grep -q 'value: default' is a faithful check of what the fixture actually declares. The removed assertions ("123456789", port 8083 probes, /nodejs/bin/node, httpGet guard) were not present anywhere in the generic fixture's values block and were therefore dead assertions that could never have passed against this fixture — or they were testing a GitHub-specific sidecar configuration that was deleted from the fixture in a prior commit. Their removal is correct and leaves no regression gap.

All remaining assertions in the connection-sidecar block are still intact and cover the meaningful invariants: image digests, volume/mount counts, security-context fields (readOnlyRootFilesystem, runAsUser), and the GitHub-App-doc leak guard.

No concerns

  • The change is purely a test cleanup — no chart templates, values schemas, or runtime code are touched.
  • The single new assertion accurately reflects the fixture values.
  • CI is green across all required checks.

@dispatch-developer
dispatch-developer Bot merged commit 46a0d3a into main Jul 20, 2026
5 checks passed
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.

1 participant