chore: Add initial base chart tests #493
Conversation
WalkthroughAdds a new GitHub Actions workflow for wandb-base snapshots and matrixed tests, generalizes snapshot tooling for multiple charts, expands wandb-base env docs and examples, adds env-focused test configs plus ConfigMap/Secret resources, introduces a chartsnap dynamic field, and adds a Helm test template for Job-type charts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant Snap as snapshots.sh
participant Helm as Helm (+plugins)
participant CT as chart-testing (ct)
participant Kind as kind
participant K8s as Test Cluster
Dev->>GH: Open PR affecting `charts/operator-wandb/**` or `test-configs/wandb-base/**`
rect rgba(220,235,255,0.35)
note right of GH: snapshots job
GH->>Helm: setup Helm & plugins
GH->>Snap: run `./snapshots.sh` (iterate charts)
Snap->>Helm: build & run chartsnap per-chart
end
rect rgba(220,255,220,0.35)
note right of GH: test job (matrix)
GH->>CT: run `ct list-changed`
GH->>GH: compute CRC32(matrix) -> cluster `ct-<hash>`
alt changes detected or ACT==true
GH->>Kind: create cluster `ct-<hash>`
GH->>Kind: apply per-config additional resources (ConfigMap/Secret) if present
GH->>CT: `ct install` with kube-context `ct-<hash>` and `test-configs/wandb-base/<config>.yaml`
CT->>K8s: install chart, run Jobs
K8s->>CT: report Job completion/logs
CT-->>GH: report results
else
GH-->>CT: skip install
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
charts/wandb-base/README.md (1)
118-126: Clarify envFrom example — document chart shorthand or show canonical PodSpecThe README shows a map-style shorthand (app-config: "configMapRef") which is not a literal PodSpec. The chart intentionally accepts this values-map shorthand and renders it in charts/wandb-base/templates/_containers.tpl (define "wandb-base.envFrom"). Update charts/wandb-base/README.md (lines 118–126) to either:
- Replace with the canonical PodSpec form:
envFrom:
- configMapRef:
name: app-config- secretRef:
name: app-secrets- Or keep the shorthand but explicitly label it as chart-specific and show the rendered PodSpec (as above) and reference templates/_containers.tpl.
🧹 Nitpick comments (15)
test-configs/additional-resources/env-values-example/configmap.yaml (1)
3-6: Scope test resources to reduce collisions.Consider adding metadata.namespace and a label (e.g., app: env-values-example) to avoid accidental cross-test/name collisions when running ct in a shared cluster or parallel jobs. Optional, but helps hermeticity.
test-configs/additional-resources/env-values-example/secret.yaml (1)
3-6: Be explicit about Secret type and masking in snapshots.
- Add type: Opaque for clarity.
- If these additional resources are ever included in snapshot output, ensure the value is masked via chartsnap dynamicFields. If not included, ignore.
snapshots.sh (3)
12-18: Gate the cascade plugin check to the build path only.Unconditionally requiring the cascade plugin will fail run/update on machines without it. Move the check inside the build case.
Suggested change (outside the shown hunk; illustrative):
# remove the global cascade check here # inside the 'build)' case, before helm cascade build: if ! helm plugin list | grep -q "^cascade\b"; then echo "❌ helm cascade plugin is not installed" echo " helm plugin install https://github.com/origranot/helm-cascade" exit 1 fi
19-30: Update usage text to reflect multi-chart behavior.Mentions “operator-wandb” only. Recommend clarifying that all charts in the list are processed and optionally documenting how to target one chart (future enhancement).
1-1: Tiny nit: stray space in shebang.There’s a trailing space after bash. Harmless, but remove for cleanliness.
test-configs/wandb-base/env-values-example.yaml (1)
47-52: Don’t assume namespace = default.The test expects USED_NAMESPACE="default". ct may install into a non-default namespace, causing false failures if this Job ever runs. Suggest making EXPECTED namespace a value (default “default”) or comparing non-empty presence only.
test-configs/wandb-base/.chartsnap.yaml (1)
1-4: Good start; consider masking additional dynamic fields.In addition to helm.sh/chart, also mask:
- metadata.annotations.meta.helm.sh/release-name
- metadata.annotations.meta.helm.sh/release-namespace
- metadata.labels.app.kubernetes.io/managed-by
- metadata.labels.app.kubernetes.io/version (if present)
- metadata.creationTimestamp (if present)
Also verify chartsnap expects jsonPath as a list vs a scalar here.
test-configs/wandb-base/env-precedence-global-env.yaml (1)
20-28: Minor: trim script verbosity.The echo lines are fine; if stdout is snapshot-compared, consider minimizing noise to reduce churn.
test-configs/wandb-base/env-precedence-chart-env.yaml (1)
26-34: Same note on output noise as other tests.Optional: keep logs terse to aid debugging in CI.
test-configs/wandb-base/env-precedence-chart-legacy.yaml (1)
23-31: Terse logs optional.Same optional suggestion to limit noisy echos.
test-configs/wandb-base/env-precedence-sizing.yaml (1)
33-41: Harden the test script for fail-fast behavior.Add shell safety flags so unexpected errors or unset vars fail the test immediately.
Apply this diff:
- | - echo "EXAMPLE=$EXAMPLE" + set -euo pipefail + echo "EXAMPLE=${EXAMPLE:-}" EXPECTED="500" if [ "$EXAMPLE" = "$EXPECTED" ]; then echo "Value is $EXPECTED" exit 0 else echo "Value is NOT $EXPECTED" exit 1 ficharts/wandb-base/README.md (2)
33-51: Minor copy edits.
- Title case: “Examples of
envat different levels” (lowercase “of”).- Consider adding a one-line reminder that container names in values must match those rendered in the chart (e.g., jobs//containers/).
Apply this diff:
-#### Examples Of `env` at different levels +#### Examples of `env` at different levels
55-66: Typo: “difference” → “different”.Small doc fix.
-At any of the difference env levels shown above the following patterns are allowed. +At any of the different env levels shown above the following patterns are allowed.test-configs/wandb-base/env-precedence-container.yaml (1)
33-44: Add shell safety flags.Mirror the hardened script approach for consistency and early failure on unset vars.
- | - echo "EXAMPLE=$EXAMPLE" + set -euo pipefail + echo "EXAMPLE=${EXAMPLE:-}" EXPECTED="600" if [ "$EXAMPLE" = "$EXPECTED" ]; then echo "Value is $EXPECTED" exit 0 else echo "Value is NOT $EXPECTED" exit 1 fitest-configs/wandb-base/env-precedence-global-legacy.yaml (1)
18-27: Add shell safety flags.Same hardening as other tests.
- | - echo "EXAMPLE=$EXAMPLE" + set -euo pipefail + echo "EXAMPLE=${EXAMPLE:-}" EXPECTED="100" if [ "$EXAMPLE" = "$EXPECTED" ]; then echo "Value is $EXPECTED" exit 0 else echo "Value is NOT $EXPECTED" exit 1 fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
test-configs/wandb-base/__snapshots__/env-precedence-chart-env.snapis excluded by!**/*.snaptest-configs/wandb-base/__snapshots__/env-precedence-chart-legacy.snapis excluded by!**/*.snaptest-configs/wandb-base/__snapshots__/env-precedence-container.snapis excluded by!**/*.snaptest-configs/wandb-base/__snapshots__/env-precedence-global-env.snapis excluded by!**/*.snaptest-configs/wandb-base/__snapshots__/env-precedence-global-legacy.snapis excluded by!**/*.snaptest-configs/wandb-base/__snapshots__/env-precedence-sizing.snapis excluded by!**/*.snaptest-configs/wandb-base/__snapshots__/env-values-example.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
.github/workflows/test-wandb-base.yaml(1 hunks)charts/wandb-base/README.md(2 hunks)snapshots.sh(2 hunks)test-configs/additional-resources/env-values-example/configmap.yaml(1 hunks)test-configs/additional-resources/env-values-example/secret.yaml(1 hunks)test-configs/wandb-base/.chartsnap.yaml(1 hunks)test-configs/wandb-base/env-precedence-chart-env.yaml(1 hunks)test-configs/wandb-base/env-precedence-chart-legacy.yaml(1 hunks)test-configs/wandb-base/env-precedence-container.yaml(1 hunks)test-configs/wandb-base/env-precedence-global-env.yaml(1 hunks)test-configs/wandb-base/env-precedence-global-legacy.yaml(1 hunks)test-configs/wandb-base/env-precedence-sizing.yaml(1 hunks)test-configs/wandb-base/env-values-example.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/test-wandb-base.yaml
47-47: "matrix values" section should not be empty
(syntax-check)
53-53: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
74-74: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
84-84: property "configuration" is not defined in object type {k8s-version: string}
(expression)
89-89: property "configuration" is not defined in object type {k8s-version: string}
(expression)
🔇 Additional comments (13)
test-configs/wandb-base/env-values-example.yaml (2)
23-33: Bytes check looks correct.1Gi -> 1073741824 via resourceFieldRef is correct. No change needed.
If you want portability, consider comparing with "$(numfmt --from=iec 1Gi)" in the script template to avoid hardcoding.
9-20: CI applies the additional resources for env-values-example — no action required.
.github/workflows/test-wandb-base.yaml and .github/workflows/test-operator-wandb.yaml apply test-configs/additional-resources/${{matrix.configuration}} before install; files exist at test-configs/additional-resources/env-values-example/configmap.yaml and secret.yaml.test-configs/wandb-base/env-precedence-global-env.yaml (1)
1-6: Precedence intent is clear.Expecting EXAMPLE=200 (global.env over global.extraEnv) matches the stated goal. Looks good.
Please confirm the chart’s merge order enforces this precedence.
test-configs/wandb-base/env-precedence-chart-env.yaml (1)
1-12: Precedence matrix reads correctly.Expecting EXAMPLE=400 (top-level env wins) is coherent given the values. No issues.
test-configs/wandb-base/env-precedence-chart-legacy.yaml (1)
1-9: Legacy precedence case is consistent.Expecting EXAMPLE=300 (top-level extraEnv) aligns with naming. LGTM.
test-configs/wandb-base/env-precedence-sizing.yaml (1)
1-19: Env precedence scenario looks correct (expects 500 from sizing.small.env).Matches the documented order: container > sizing > chart env > extraEnv > global.env > global.extraEnv.
Please confirm this aligns with the actual render logic in the chart templates (helpers) for env precedence.
test-configs/wandb-base/env-precedence-container.yaml (1)
1-19: Env precedence plus explicit container override looks correct (expects 600 from container env).Scenario covers the top-most precedence layer.
test-configs/wandb-base/env-precedence-global-legacy.yaml (1)
1-4: Covers legacy global.extraEnv path.Good to have a dedicated test for legacy behavior.
.github/workflows/test-wandb-base.yaml (4)
21-39: Snapshot step: confirm target chart and intentional skip of chartsnap action.
- You add repos inside charts/operator-wandb but run snapshots.sh from repo root; confirm script paths.
- The chartsnap action is permanently skipped via
if: ${{ !always() }}; ensure that’s intentional.Do you want snapshots for wandb-base here? If so, set the chart path accordingly and enable the action.
81-86: Verified: kindest/node tags exist for matrix versionsv1.32.2, v1.31.6, and v1.30.10 are included in the kind v0.27.0 release — pin to their @sha256 digests from the release to lock exact images.
80-87: The “test” job currently performs no lint/install; enable ct lint/install.As written, it only lists changes and (optionally) creates a cluster. Enable at least lint; optionally install with your test values.
+ - name: Run chart-testing (lint) + if: env.ACT || steps.list-changed.outputs.changed == 'true' + run: | + ct lint --config ct.yaml @@ - #- name: Run chart-testing (install) - # env: - # LICENSE: ${{ secrets.LICENSE }} - # if: env.ACT || steps.list-changed.outputs.changed == 'true' - # run: | - # ct install --namespace default \ - # --charts ./charts/operator-wandb \ - # --config ct.yaml \ - # --helm-extra-args '--kube-context kind-chart-testing-${{ matrix.k8s-version }}-${{ matrix.configuration }} --timeout 600s' \ - # --helm-extra-set-args '--values test-configs/operator-wandb/${{ matrix.configuration }}.yaml --set=license=$LICENSE' + - name: Run chart-testing (install) + if: env.ACT || steps.list-changed.outputs.changed == 'true' + run: | + ct install --namespace default \ + --config ct.yaml \ + --helm-extra-args '--kube-context kind-chart-testing-${{ matrix.k8s-version }}${{ matrix.configuration && format("-%s", matrix.configuration) || "" }} --timeout 600s'Adjust charts/values selection flags to point at wandb-base and the specific test-configs you intend to validate.
Also applies to: 97-107
45-49: Remove or populate the emptyconfigurationmatrix axis
Emptyconfigurationyields no job combinations (andmatrix.configurationis undefined). Either delete theconfigurationaxis and strip its references, or fill it with your actual test‐config names. Apply the same change at lines 84–86 and 89–95.charts/wandb-base/README.md (1)
58-60: Verified — k8s links are correct for stable v1.34Verified: current stable Kubernetes minor version is 1.34 (latest 1.34.1, released 2025-09-09). The EnvVar and EnvVarSource API reference URLs under /docs/reference/generated/kubernetes-api/v1.34/ are correct.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
charts/wandb-base/templates/tests/test-jobs.yaml (4)
18-18: Pin kubectl image; avoid latest.latest is mutable and risks supply-chain drift. Pin to a version or digest, ideally configurable via values.
- image: bitnami/kubectl:latest + image: bitnami/kubectl:1.30.2 +# or: +# image: bitnami/kubectl@sha256:<digest>Optionally expose tag via .Values.test.kubectlImageTag.
17-22: Harden test Pod securityContext.Even for tests, drop privileges to meet baseline policies.
- name: job-test image: bitnami/kubectl:1.30.2 command: ['/bin/sh'] args: - -c - | + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault + capabilities: + drop: ["ALL"]
12-14: Auto-clean up test Pods after success.Add hook-delete-policy to reduce cluster litter during CI.
annotations: "helm.sh/hook": test "helm.sh/hook-weight": "1" + "helm.sh/hook-delete-policy": "before-hook-creation,hook-succeeded"
15-48: RBAC can block kubectl in restricted clusters.If clusters enforce RBAC, default SA may lack get/describe/logs on Jobs/Pods. Consider adding a minimal Role/RoleBinding and setting serviceAccountName for the test Pod.
.github/workflows/test-wandb-base.yaml (2)
22-30: Snapshot step targets operator-wandb; confirm scope.If this workflow is for wandb-base, either point snapshots to wandb-base or clarify multi-chart scope.
- pushd ./charts/operator-wandb/ + pushd ./charts/wandb-base/Or parameterize snapshots.sh to accept chart path(s) and call for both charts as needed.
9-39: Least-privilege and cleanup nits (optional).
- Add top-level permissions: contents: read.
- Drop the disabled chartsnap step to reduce noise (or guard with an input flag).
name: Test wandb-base Chart +permissions: + contents: read
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/test-wandb-base.yaml(1 hunks)charts/wandb-base/templates/tests/test-jobs.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/test-wandb-base.yaml
60-60: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
81-81: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
🔇 Additional comments (3)
.github/workflows/test-wandb-base.yaml (3)
1-8: Align workflow name and path triggers with wandb-base.Name says operator-wandb and paths exclude charts/wandb-base/**, while the job installs ./charts/wandb-base. Align intent.
-name: Test operator-wandb Chart +name: Test wandb-base Chart @@ pull_request: paths: - - charts/operator-wandb/** + - charts/wandb-base/** - test-configs/wandb-base/**If snapshots should also cover operator-wandb, reflect both charts in name and paths explicitly.
60-60: Upgrade checkout to v4.v3 is deprecated on GH-hosted runners.
- uses: actions/checkout@v3 + uses: actions/checkout@v4
81-85: Replace deprecated set-output with GITHUB_OUTPUT.run: | changed=$(ct list-changed --config ct.yaml) if [[ -n "$changed" ]]; then - echo "::set-output name=changed::true" + echo "changed=true" >> "$GITHUB_OUTPUT" fi
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/workflows/test-wandb-base.yaml (4)
22-26: Remove unnecessary pushd/popd and fix chart path confusion.You’re adding repos; cwd doesn’t matter. Also this workflow targets wandb-base, not operator-wandb.
- pushd ./charts/operator-wandb/ helm repo add bitnami https://charts.bitnami.com/bitnami helm repo add stakater https://stakater.github.io/stakater-charts helm repo add prometheus https://prometheus-community.github.io/helm-charts - popd
27-30: Pin Helm plugin versions and make installs idempotent.Unpinned plugin installs are non-reproducible and can flake. Guard install and pin versions. Replace placeholders with known tags.
- helm plugin install https://github.com/origranot/helm-cascade - helm plugin install https://github.com/jlandowner/helm-chartsnap + helm plugin list | grep -q 'cascade' || helm plugin install https://github.com/origranot/helm-cascade --version vX.Y.Z + helm plugin list | grep -q 'chartsnap' || helm plugin install https://github.com/jlandowner/helm-chartsnap --version vA.B.C
31-39: Prefer explicit disable over!always().
if: ${{ false }}(or comment the step) reads clearer than negatingalways().- - name: Chart Snapshots - if: ${{ !always() }} + - name: Chart Snapshots + if: ${{ false }}
1-8: Optional: add concurrency to auto-cancel superseded PR runs.Prevents wasting CI minutes on outdated commits.
name: Test wandb-base Chart on: pull_request: paths: - charts/operator-wandb/** - test-configs/wandb-base/** + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test-wandb-base.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/test-wandb-base.yaml
59-59: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
80-80: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
88-88: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
🔇 Additional comments (4)
.github/workflows/test-wandb-base.yaml (4)
3-8: Align triggers with intent (wandb-base).Workflow name targets wandb-base, but
on.pull_request.pathswatchescharts/operator-wandb/**. Switch tocharts/wandb-base/**(or include both and rename workflow accordingly).on: pull_request: paths: - - charts/operator-wandb/** + - charts/wandb-base/** - test-configs/wandb-base/**
78-85: Use GITHUB_OUTPUT instead of deprecated ::set-output.Modernize step outputs.
- name: Run chart-testing (list-changed) id: list-changed run: | changed=$(ct list-changed --config ct.yaml) if [[ -n "$changed" ]]; then - echo "::set-output name=changed::true" + echo "changed=true" >> "$GITHUB_OUTPUT" fi
59-61: Update checkout action to v4.
actions/checkout@v3is outdated on GH-hosted runners.- - name: Checkout - uses: actions/checkout@v3 + - name: Checkout + uses: actions/checkout@v4 with: fetch-depth: 0
46-54: Invalid YAML list syntax for matrix.configuration (workflow won’t parse).Convert to a block sequence.
- configuration: [ - env-values-example - env-precedence-container - env-precedence-sizing - env-precedence-chart-env - env-precedence-chart-legacy - env-precedence-global-env - env-precedence-global-legacy, - ] + configuration: + - env-values-example + - env-precedence-container + - env-precedence-sizing + - env-precedence-chart-env + - env-precedence-chart-legacy + - env-precedence-global-env + - env-precedence-global-legacy
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
snapshots.sh (1)
25-29: Update usage text to reflect multi-chart support.Clarify that build operates on the selected chart(s).
Apply:
Commands: - build, build the operator-wandb chart + build, build the selected chart (or all default charts) update, execute chartsnap to update the snapshots run, executes chartsnap to test helm changes
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
test-configs/wandb-base/__snapshots__/env-precedence-chart-env.snapis excluded by!**/*.snaptest-configs/wandb-base/__snapshots__/env-precedence-chart-legacy.snapis excluded by!**/*.snaptest-configs/wandb-base/__snapshots__/env-precedence-container.snapis excluded by!**/*.snaptest-configs/wandb-base/__snapshots__/env-precedence-global-env.snapis excluded by!**/*.snaptest-configs/wandb-base/__snapshots__/env-precedence-global-legacy.snapis excluded by!**/*.snaptest-configs/wandb-base/__snapshots__/env-precedence-sizing.snapis excluded by!**/*.snaptest-configs/wandb-base/__snapshots__/env-values-example.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
.github/workflows/test-wandb-base.yaml(1 hunks)snapshots.sh(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/test-wandb-base.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Chart (v1.30.10, env-values-example)
🔇 Additional comments (2)
snapshots.sh (2)
38-43: Guard chartsnap -f when values dir is missing.Avoid failing charts without test-configs; skip or run without -f.
Apply:
function update_chart() { local chart="$1" local values_file="$2" - echo "updating $chart snapshots" - helm chartsnap -c "./charts/$chart" -u -f "$values_file" + echo "Updating $chart snapshots" + if [[ -d "$values_file" ]]; then + helm chartsnap -c "./charts/$chart" -u -f "$values_file" + else + echo "No values dir $values_file; skipping chartsnap update for $chart" + fi }
45-50: Guard chartsnap -f when values dir is missing.Same issue in run path.
Apply:
function run_chart() { local chart="$1" local values_file="$2" - echo "Checking $chart snapshot tests" - helm chartsnap -c "./charts/$chart" -f "$values_file" + echo "Checking $chart snapshot tests" + if [[ -d "$values_file" ]]; then + helm chartsnap -c "./charts/$chart" -f "$values_file" + else + echo "No values dir $values_file; skipping chartsnap tests for $chart" + fi }
| local func="$1" | ||
| local charts=("wandb-base" "operator-wandb") | ||
| local values_dir="test-configs" | ||
|
|
||
| if [ $# -eq 0 ]; then | ||
| usage | ||
| exit 1 | ||
| fi | ||
|
|
||
| local func="$1" | ||
| case "$func" in | ||
| build) | ||
| echo "Building operator-wandb" | ||
| helm cascade build "./charts/$chart" | ||
| ;; | ||
| update) | ||
| echo "Updating operator-wandb snapshots" | ||
| helm chartsnap -c "./charts/$chart" -u -f "./$values_dir/$chart" | ||
| ;; | ||
| run) | ||
| echo "Checking snapshot tests" | ||
| helm chartsnap -c "./charts/$chart" -f "./$values_dir/$chart" | ||
| ;; | ||
| *) | ||
| usage | ||
| exit 1 | ||
| ;; | ||
| esac | ||
| if [ -n "$2" ]; then | ||
| charts=("$2") | ||
| fi | ||
|
|
There was a problem hiding this comment.
Fix set -u unbound args ($1/$2) in main.
Referencing $1/$2 before validating args breaks under set -u.
Apply:
-function main() {
- local func="$1"
- local charts=("wandb-base" "operator-wandb")
- local values_dir="test-configs"
+function main() {
+ # Ensure at least one argument before referencing $1 under set -u
+ if [ "$#" -lt 1 ]; then
+ usage
+ exit 1
+ fi
+ local func="$1"
+ shift
+ local charts=("wandb-base" "operator-wandb")
+ local values_dir="test-configs"
@@
- if [ -n "$2" ]; then
- charts=("$2")
- fi
+ if [ "$#" -ge 1 ]; then
+ charts=("$1")
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local func="$1" | |
| local charts=("wandb-base" "operator-wandb") | |
| local values_dir="test-configs" | |
| if [ $# -eq 0 ]; then | |
| usage | |
| exit 1 | |
| fi | |
| local func="$1" | |
| case "$func" in | |
| build) | |
| echo "Building operator-wandb" | |
| helm cascade build "./charts/$chart" | |
| ;; | |
| update) | |
| echo "Updating operator-wandb snapshots" | |
| helm chartsnap -c "./charts/$chart" -u -f "./$values_dir/$chart" | |
| ;; | |
| run) | |
| echo "Checking snapshot tests" | |
| helm chartsnap -c "./charts/$chart" -f "./$values_dir/$chart" | |
| ;; | |
| *) | |
| usage | |
| exit 1 | |
| ;; | |
| esac | |
| if [ -n "$2" ]; then | |
| charts=("$2") | |
| fi | |
| function main() { | |
| # Ensure at least one argument before referencing $1 under set -u | |
| if [ "$#" -lt 1 ]; then | |
| usage | |
| exit 1 | |
| fi | |
| local func="$1" | |
| shift | |
| local charts=("wandb-base" "operator-wandb") | |
| local values_dir="test-configs" | |
| if [ "$#" -ge 1 ]; then | |
| charts=("$1") | |
| fi |
🤖 Prompt for AI Agents
In snapshots.sh around lines 53 to 65, the script assigns local func="$1" and
later checks $# which causes unbound variable errors when run with set -u; move
any direct references to positional parameters until after you validate
arguments (e.g., perform the if [ $# -eq 0 ] check and exit before assigning
local func and reading $2), or use safe parameter expansion like "${1:-}" and
"${2:-}" when you must reference them earlier; update the code so no plain $1/$2
is referenced before argument validation to avoid set -u failures.
Summary by CodeRabbit
Documentation
Tests
New Features
Chores