test(complex): add helm-unittest suites and testing-values for recent…#21
Merged
Conversation
… features Add unit tests for the three most recent features that were merged without dedicated tests: Recreate deployment strategy, nodeName propagation, and hostNetwork/dnsPolicy. Tests live in complex/tests/ and run via the helm-unittest plugin. Also add three new files in complex/testing-values/ that exercise the same features end-to-end through helm template, and wire all of them into both the lint and template-test CI jobs. A new unit-test job runs the helm-unittest suites against the chart on every PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ures Extend the helm-unittest coverage to three older features that have non-trivial branching in templates: - PVC: defaults, custom storageClass/accessModes/annotations/labels, PVC creation skipped when useExistingClaim is true, and pod spec claimName resolution for both new and existing claims. - PDB: integer vs percentage values for minAvailable/maxUnavailable, extraMatchLabels, naming convention. - Standalone ingress component: 'ingress' type does not produce Deployment/Service/HPA/ServiceAccount, port vs servicePort alias, externalService backend routing, TLS rendering. 21 new tests across 3 suites; total now 36 tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ometheus Cover the remaining recent features that were missing unit tests: - initContainers: defaults, single/multiple containers, resource propagation, no leakage into the main containers list. - Traefik Middleware: global vs component naming, ingress ref resolution for plain refs, global: refs and external @-refs, multi-ref joining, conflict guard against manual annotation. - KEDA Prometheus trigger: metricName vs query branches, float thresholds, all optional fields, schema enforcement of required serverAddress / metricName-or-query / threshold. 20 new tests across 3 suites; total now 56 tests across 9 suites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…es + tests
Three latent template bugs that were never exercised by existing tests:
1. serviceaccount.yaml line 5 used '--' instead of '---' as YAML
document separator. Any user setting both serviceAccountCreate and
serviceAccountName on a component hit a helm-time YAML parse error.
2. serviceaccount.yaml whitespace stripping on the global if block
('-}}') ate the newline before the second '---', producing
'keep---' glued together when both component-level and global SAs
were configured.
3. keda-scaledobject.yaml dereferenced '.Values.global.keda.awsRegion'
eagerly inside `default`, causing a nil pointer error when
global.keda was unset (rather than the intended explicit fail
message about a missing required field). Fixed with parens-guarded
access ($.Values.global.keda).awsRegion.
Add four new unit test suites locking in the fixes and covering
features that previously had no positive coverage:
- service_account_test: 9 tests, including both-SAs-configured and
the cronjob/pre-job/ingress exclusions that prevented the bug from
being noticed.
- hpa_test: 8 tests for positive HPA rendering, defaults, custom
thresholds and stabilization windows.
- keda_kafka_sqs_test: 12 tests for Kafka SASL/awsRegion fallback,
identityOwner branches, SQS authenticationRef handling, cron
trigger, and unknown-trigger schema rejection.
- target_group_binding_test: 7 tests for single/multi targetGroups,
default-name suffix omission, ingress exclusion, non-http
exclusion.
Bump chart version 1.8.3 -> 1.8.4 (patch bump for the fixes). README
regenerated via helm-docs.
Total now 92 tests across 13 suites.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… PrometheusRule Cover the remaining templates that previously had no positive unit-test coverage: - ConfigMap: standard labels, custom labels merging, annotations, immutable flag, tpl substitution of Release.Name in data. - VPA: defaults for updateMode and min/max CPU/memory, custom values, wildcard containerName, exclusions for cronjob/pre-job. - KEDA TriggerAuthentication: rendered only for SQS triggers with pod identityOwner, indexed naming per trigger position, skipped for operator identity and non-SQS trigger types. - CronJob: schedule, Forbid concurrency policy, default/honoured suspend, backoffLimit on inner Job template. - pre-job: helm hook annotations (pre-install/pre-upgrade, hook-delete-policy, hook-weight default 0 vs custom), restartPolicy Never, backoffLimit 0. - PrometheusRule: alertRules with default for=1m and severity=critical, global alertLabels merging, alertTemplates with environment/project substitution and default-env/default-project fallbacks. 43 new tests across 5 suites; total now 135 tests across 18 suites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tion Two paired changes that resurrect the triggerAuthEnabled feature: - values.schema.json: register triggerAuthEnabled (boolean, default true) on the kedaTriggerAwsSqsQueue trigger. Previously the schema rejected the property as 'Additional property not allowed', so the feature was unreachable through legitimate values. - keda-triggerauthentication.yaml: replace `default true X` guard with `ne X false`. Go template's `default` treats a literal false as empty and falls back to true, so triggerAuthEnabled: false had no effect even bypassing the schema. The new guard returns true unless the property is explicitly false. Adds back the previously-removed unit test exercising triggerAuthEnabled: false. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Move PVC documentation into README.md.gotmpl so it survives helm-docs regeneration. The previous README contained a manually added PVC section that lived only in README.md (added in #14) and was lost when README was regenerated for the version bump. - Drop redundant `and` with a single argument in serviceaccount.yaml's global SA guard. - Consolidate the duplicated strategyType schema into a single definitions/strategyType entry referenced via $ref from both global and per-component schemas, matching the existing pattern used for rollingUpdate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nly bug Two new helm-unittest suites: - service_test (13 tests): positive coverage of templates/service.yaml. Renders for http and http-internal, skipped for consumer/cronjob/ pre-job, default ClusterIP, component-level and global-level service type override, single and multiple ports, selector labels, and one Service per http-type component. Service template previously had no dedicated suite. - volume_mount_readonly_test (10 tests): locks in the readOnly: false behaviour for configMap and secret volume mounts. Default true is preserved when unset, but explicit false now propagates correctly (regression for the default-true Go template bug fixed in this commit). emptyDirs and others mounts also covered. Paired template fix in _container.tpl: replace `default true .readOnly` with `ne .readOnly false` for configMap and secret volume mounts. Same class of bug as the recently-fixed triggerAuthEnabled - Go template's `default` treats literal false as empty and falls back to true, so users who set readOnly: false silently got readOnly: true. Verified the new regression tests fail against the buggy template before applying the fix. Total now 159 tests across 20 suites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jindraj
requested changes
Apr 29, 2026
jindraj
left a comment
jindraj
left a comment
Contributor
There was a problem hiding this comment.
First round. I focused solely on package versions, all the newly added packages in.github/workflows/ci.yaml are already old.
I'll look on the rest later, it will take me some time to compare outputs of the tests with previous runs.
Contributor
|
Outputs of helm template tests are identical, only the newly added tests are in diff. Changes The helm template tests doesn't cover all cases, e.g.: the fixed yaml document separation in |
Cosmetic cleanup - had a trailing-space line between two helm template blocks left over from an earlier edit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The if/else duplicated `strategy:` and `type:` declarations on both branches just to toggle the rollingUpdate sub-block. Hoist the strategy block out and gate only the rollingUpdate inner block. Same rendered output: - default / RollingUpdate: strategy.type + rollingUpdate present - Recreate: strategy.type only recreate_strategy_test continues to pass (4 tests covering both strategies and per-component independence). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New helm-unittest suite locking in the core Deployment rendering paths that were previously only exercised indirectly through feature tests: - replicas, progressDeadlineSeconds (default 300 + override) - container image (global default + per-component repository/tag/ pullPolicy override) - command, args (rendered when set, omitted when unset) - ports as containerPort entries - env values (literal envs.values) - env valueFrom for fieldRef short-form, secretKeyRef - envFrom for secretRef and configMapRef - resources (global inheritance + per-component override) - liveness/readiness/startup probes - restartPolicy default Always, terminationGracePeriodSeconds default 120 - enableServiceLinks default False + True override - imagePullSecrets (global propagation, component override, omission) - tolerations (rendered + omitted when unset) - nodeSelector merging global + component - selector matchLabels match pod template labels 25 new tests; total now 184 across 21 suites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Jakub Jindra <jakub.jindra@cookielab.io>
Co-authored-by: Jakub Jindra <jakub.jindra@cookielab.io>
Co-authored-by: Jakub Jindra <jakub.jindra@cookielab.io>
Co-authored-by: Jakub Jindra <jakub.jindra@cookielab.io>
The existing 'creates both component and global SAs' case caught the '-}}' whitespace bug that glued the global SA into the component SA document, but only because both line 5 and line 20 separators were exercised together. Removing only one '---' (e.g. just line 5 in serviceaccount.yaml) was not detected by any test, because helm prepends its own '---' at the start of a single-document file. Two new cases plug that gap: - renders two SAs as separate documents when two components both create one (api + worker consumer). Without the line-5 separator, the second component's document is appended to the first, hitting 'apiVersion already defined'. - renders three documents when two components and global all create SAs. Catches both line-5 and line-20 separators independently. Verified empirically by deleting each separator in turn: - delete line 5 only: 2 new tests fail (was: 0 of 9 caught it) - delete line 20 only: 1 existing + 1 new test fail - delete both: 1 existing + 2 new tests fail Total now 186 tests across 21 suites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rmat helm-unittest 1.0.3 (now used in CI) ships with a newer JSON schema validator that emits 'missing property X' / ''anyOf' failed' instead of the old 'X is required' / 'Must validate at least one schema'. The schema-rejection tests in keda_prometheus_trigger_test and keda_kafka_sqs_test were written against the old format and started failing on CI after the unit-test job upgrade. Make the errorPattern regex accept either format so the tests pass on both old (0.6.x) and new (1.0.x) helm-unittest versions. Verified locally with both 0.6.2 and 1.0.3 - all 186 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
… features
Add unit tests for the three most recent features that were merged without dedicated tests: Recreate deployment strategy, nodeName propagation, and hostNetwork/dnsPolicy. Tests live in complex/tests/ and run via the helm-unittest plugin.
Also add three new files in complex/testing-values/ that exercise the same features end-to-end through helm template, and wire all of them into both the lint and template-test CI jobs. A new unit-test job runs the helm-unittest suites against the chart on every PR.