test(bdd): group non-empty YAML key assertions - #1058
Conversation
Add a table-driven YAML assertion that distinguishes missing keys from empty values, and migrate repeated registration-output checks across local and EKS features. Closes #863 Signed-off-by: Stephanie Baum <sbaum@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (13)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe BDD DSL adds a table-driven assertion for non-empty YAML keys. The assertion validates table structure, reports key-specific errors, and replaces repeated registration-value checks across feature scenarios. ChangesGrouped YAML assertions
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change only improves BDD YAML assertion grouping and error reporting without changing customer-facing or runtime behavior; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant BDDScenario
participant YAMLAssertionStep
participant RequireNonEmptyYAMLKeys
participant ReadYAMLKey
BDDScenario->>YAMLAssertionStep: provide YAML path and key table
YAMLAssertionStep->>RequireNonEmptyYAMLKeys: validate listed keys
RequireNonEmptyYAMLKeys->>ReadYAMLKey: read each dotted key
ReadYAMLKey-->>RequireNonEmptyYAMLKeys: return value or error
RequireNonEmptyYAMLKeys-->>YAMLAssertionStep: return validation result
YAMLAssertionStep-->>BDDScenario: pass or report assertion error
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
TL;DR
Add one table-driven YAML assertion for non-empty keys and replace repeated registration-output checks across eight BDD features. The feature files still show every key being checked, while missing and empty values now report the failing table row.
Additional Details
Why
Registration scenarios repeated the same long file path for each generated ID. Grouping those assertions improves readability without hiding the file, key names, or expected non-empty behavior.
What changed
yaml file "..." should have non-empty keys:with a requiredkeycolumn.Customer Release Notes
Not customer visible.
Plan Summary
Not applicable. This changes test code and feature wording only.
Usage
Notes
Live-run issue log:
RequestICMSInstancesForTaskand completed the task. A full monorepo-built image instead returned a task-read 404 after artifact validation; that separate QA finding is documented on Self-managed NVCT tasks use the function ICMS request action #1032 and fix(nvct): request task instances from SIS #1042.No runtime observability or architecture behavior changes.
References
Related Pull Requests
Dependencies
None. No license review or NOTICE update is required.
For the Reviewer
Please focus on the table validation and row-specific errors in
tests/bdd/steps/assertion_steps.goandtests/bdd/dsl/yamledit.go.For QA
go test -short ./...: passedgolangci-lint run --config .golangci.yml ./...: passed with 0 issuesTestSingleClusterUp: 3 scenarios, 51 steps passedTestSingleClusterHelmfile: 5 scenarios, 48 steps passedTestObservabilityAll: 1 scenario, 37 steps passedTestMultiClusterUp: 4 scenarios, 63 steps passedTestMultiClusterHelmfile: 5 scenarios, 68 steps passed; see the NVCT note aboveTestObservabilityCompute: 1 scenario, 45 steps passedAdditional QA is not required for this test-only DSL change.
Issues
Closes #863
Relates to #858
Checklist
Summary by CodeRabbit
New Features
Tests