test(tests): make absence-only helm-unittest cases fail when they check nothing by lexfrei · Pull Request #4664 · cozystack/cozystack

Aleksei Sviridkin (lexfrei) · 2026-10-01T19:33:49Z

What this PR does

Twenty-one helm-unittest cases asserted only that something is absent from a document, and could pass without ever looking at the document they name. This PR makes each of them fail when that happens.

Thirteen cases picked their document by position: mongodb (4), nats certmanager (3), opensearch (3), qdrant certmanager (2), seaweedfs cluster_scoped_names (1). When a template gains or reorders a document, the index points at some other object that also lacks the field, and the case stays green. These now use documentSelector on kind or metadata.name, like the earlier harbor conversion.
Seven cases had no selector at all, on templates wrapped in a top-level if: kubernetes cilium_gateway_alpn, mongodb restore, linstor-scheduler admission, seaweedfs s3_traffic_distribution and three in vertical-pod-autoscaler. If a values change turns the condition off, the render is empty and the absence check passes on nothing. A documentSelector turns an empty render into document not found.
The redis null-namespace case checked that the in-cluster name is present, but never that the external name is absent. It now asserts that neither my-redis. nor my-redis.example.com (the suite-level host that _namespace: null must override) is in the SAN list.

The kubernetes cluster entry in the #3634 catalog no longer applies, since packages/apps/kubernetes/tests/cluster_test.yaml has been removed from main. I did not add the my-redis.redis. check that #4621 mentions, because the per-engine SAN it refers to has not landed.

Each case was checked by mutation. I broke the template in a way the old case could not see, then ran the chart's make test with the new test file and with the one from main. For group 1 I removed the guard and put an extra document in front of the target. For group 2 I turned off the top-level condition. For group 3 I let the empty-host branch emit the bare SAN. Every time the main version stayed green and the new one went red, and the fixed cases were the only extra failures. All templates are back to their committed bytes.

Follow-ups, not in this PR:

The same absence-only shape has appeared in more cases since testing: 14 test cases in 7 suites pin documents by index with only negative asserts, so they cannot see index drift #3634 was filed: mongodb tls_test.yaml (8 cases), mariadb tls_test.yaml (6), redis tls_test.yaml (2 more), kubernetes kamaji_managed_args_test.yaml (2), and one each in kafka dashboard-resourcemap_test.yaml, cert-manager-issuers solver_test.yaml and seaweedfs volume_resize_hook_test.yaml. They need a separate issue. The four platform bundles_proxy_protocol* cases are testing: negated containsDocument without any: true passes on any multi-document render #3864, handled in fix(platform tests): make negated containsDocument assertions load-bearing with any: true #3929.

Closes #3634

Closes #4621

Screenshots

Not applicable, test-only change.

Downstream repositories

Release note

NONE

Summary by CodeRabbit

Tests
- Updated chart validation to identify rendered resources by their type or name rather than relying on document order, improving consistency across Kubernetes applications.
- Expanded Redis TLS checks to verify that certificates omit generic and external hostnames when no namespace data is provided.
- Kept existing expected behavior checks for gateway, database, messaging, search, storage, and autoscaling resources.

The null-namespace case only checked that the in-cluster name is present, so a chart that emitted a bare "my-redis." SAN, or fell back to the suite-level host, would still pass. It now carries the same absence check as the empty-host case, plus the suite host itself. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>

These cases picked a document by position and then asserted only that something is absent from it. When a template gains or reorders a document, the index lands on a different object that lacks the field just as well, and the case stays green while the property it names is no longer checked. Selecting by kind or metadata.name keeps each case on the object it names, and fails with "document not found" when that object stops rendering. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>

Each of these templates is wrapped in a top-level condition, and the case asserts only that a field is absent. If a values change turns the condition off, the render is empty and the absence holds trivially, so the case passes without looking at anything. A document selector makes an empty render fail with "document not found". Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>

coderabbitai · 2026-10-01T19:34:20Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: cozystack/cozystack/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 71983b66-48db-47ab-b132-1bf92aeb2665

📥 Commits

Reviewing files that changed from the base of the PR and between bc400df and fccb7fe.

📒 Files selected for processing (12)

packages/apps/kubernetes/tests/cilium_gateway_alpn_test.yaml
packages/apps/mongodb/tests/mongodb_test.yaml
packages/apps/mongodb/tests/restore_test.yaml
packages/apps/nats/tests/certmanager_test.yaml
packages/apps/opensearch/tests/opensearch_test.yaml
packages/apps/opensearch/tests/security_test.yaml
packages/apps/qdrant/tests/certmanager_test.yaml
packages/apps/redis/tests/tls_test.yaml
packages/system/linstor-scheduler/tests/admission_test.yaml
packages/system/seaweedfs/tests/cluster_scoped_names_test.yaml
packages/system/seaweedfs/tests/s3_traffic_distribution_test.yaml
packages/system/vertical-pod-autoscaler/tests/vpa_test.yaml

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.

📝 Walkthrough

Walkthrough

The PR updates Helm unit tests to select rendered resources by kind or name instead of document position in several application and system charts. It also adds two absent-SAN assertions to the Redis null-namespace TLS test. Existing assertions remain unchanged where selectors changed.

Changes

Helm test assertions

Layer / File(s)	Summary
Application resource selection `packages/apps/kubernetes/tests/cilium_gateway_alpn_test.yaml`, `packages/apps/mongodb/tests/`, `packages/apps/nats/tests/certmanager_test.yaml`, `packages/apps/opensearch/tests/`, `packages/apps/qdrant/tests/certmanager_test.yaml`	Tests select the expected rendered resource by kind or `metadata.name` instead of document index. Existing assertions remain unchanged.
System resource selection `packages/system/linstor-scheduler/tests/admission_test.yaml`, `packages/system/seaweedfs/tests/*`, `packages/system/vertical-pod-autoscaler/tests/vpa_test.yaml`	Tests select the expected rendered resource by kind before applying existing assertions.
Redis TLS SAN assertions `packages/apps/redis/tests/tls_test.yaml`	The null-namespace TLS test now checks that `my-redis.` and `my-redis.example.com` are absent from the leaf Certificate’s `spec.dnsNames`.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Other · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to fccb7

The test updates appear to check the intended resources and SAN exclusions, with no material merge risk identified.

Architecture Summary

Architecture risk: 🔵 Low · up to fccb7

The change affects 2 systems.

Changed systems: packages/apps, packages/system

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

observed — packages/apps (library) was modified; 8 changed files map to changed impact.
observed — packages/system (library) was modified; 4 changed files map to changed impact.

Before / after behavior

observed — Modified behavior in packages/apps/kubernetes/tests/cilium_gateway_alpn_test.yaml: The disabled-addon test now targets the document whose kind is HelmRelease.
observed — Modified behavior in packages/apps/mongodb/tests/mongodb_test.yaml: The replica-count-3 assertion still checks that spec.unsafeFlags is absent, but now selects the PerconaServerMongoDB document by kind instead of assuming document index 0.
observed — Modified behavior in packages/apps/mongodb/tests/mongodb_test.yaml: The replica-count-5 assertion still checks that spec.unsafeFlags is absent, but now selects the PerconaServerMongoDB document by kind instead of assuming document index 0.
observed — Modified behavior in packages/apps/mongodb/tests/mongodb_test.yaml: The empty-storage-class assertion still checks that the storage class field is absent, but now selects the PerconaServerMongoDB document by kind instead of assuming document index 0.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: strengthening absence-only Helm-unittest cases so they fail when they check no document or assertion target.
Linked Issues check	✅ Passed	The PR converts the 13 applicable cases from `#3634` to `documentSelector` checks. The changes cover MongoDB, NATS, OpenSearch, Qdrant, and SeaweedFS. The Kubernetes case is no longer applicable because…
Out of Scope Changes check	✅ Passed	The changes are limited to Helm-unittest test assertions. The 13 selector conversions implement `#3634`. The seven additional selector conversions address the same document-selection failure mode. The R…
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…

✨ Finishing Touches

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

myasnikovdaniil

Looks good, nothing blocking.

Two more cases in the redis tls_test.yaml (around lines 550 and 827) have the shape this PR removes, documentIndex: 3 with only notContains (external=false SAN list and external LB name). With a decoy document in front and the external guard forced on they stay green. Two selectors here would cover them, otherwise they need the follow-up issue from the description, none linked yet.

myasnikovdaniil · 2026-10-02T10:52:44Z

+          content: "my-redis."
+      - notContains:
+          path: spec.dnsNames
+          content: "my-redis.example.com"


Closing #4621 drops its second half, the my-redis.redis. check. #4627 adds that notContains to the empty-host case only, so once both land this null-namespace case has no check for it. Either Refs here and keep the issue open, or add the assert here after that PR merges.

Aleksei Sviridkin (lexfrei) added 3 commits October 1, 2026 22:23

Aleksei Sviridkin (lexfrei) requested review from IvanHunters, Andrei Kvapil (kvaps), Timofei Larkin (lllamnyp), Matthieu ROBIN (matthieu-robin), mattia-eleuteri, myasnikovdaniil, scooby87 and Kirill Ilin (sircthulhu) as code owners October 1, 2026 19:33

github-actions Bot added area/testing Issues or PRs related to testing (e2e, bats, unit tests) size/M This PR changes 30-99 lines, ignoring generated files labels Oct 1, 2026

This was referenced Oct 1, 2026

testing: about 21 more helm-unittest cases select a document by position or assert absence with no selector #4665

Open

[Tracking] Shell test architecture: vanilla BATS for unit tests, assert library, testable suite shell #3453

Open

myasnikovdaniil approved these changes Oct 2, 2026

View reviewed changes

Aleksei Sviridkin (lexfrei) merged commit 10a6cf2 into main Oct 2, 2026
39 of 41 checks passed

Aleksei Sviridkin (lexfrei) deleted the test/helm-asserts-that-can-fail branch October 2, 2026 11:08

Navigation Menu

Repository navigation

test(tests): make absence-only helm-unittest cases fail when they check nothing - #4664

test(tests): make absence-only helm-unittest cases fail when they check nothing#4664
Aleksei Sviridkin (lexfrei) merged 3 commits into
mainfrom
test/helm-asserts-that-can-fail

Aleksei Sviridkin (lexfrei) commented Oct 1, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 1, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

myasnikovdaniil left a comment

Uh oh!

myasnikovdaniil Oct 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Aleksei Sviridkin (lexfrei) commented Oct 1, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Screenshots

Downstream repositories

Release note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Architecture Summary

Uh oh!

myasnikovdaniil left a comment

Choose a reason for hiding this comment

Uh oh!

myasnikovdaniil Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Aleksei Sviridkin (lexfrei) commented Oct 1, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 1, 2026 •

edited

Loading