Skip to content

fix(tests): strengthen UAT recipe and validation assertions#537

Open
ayuskauskas wants to merge 1 commit intomainfrom
feat/uat-test-enhancement
Open

fix(tests): strengthen UAT recipe and validation assertions#537
ayuskauskas wants to merge 1 commit intomainfrom
feat/uat-test-enhancement

Conversation

@ayuskauskas
Copy link
Copy Markdown
Contributor

Replace trivially-passing UAT assertions with full structural checks. Recipe assertions now verify every expected component by name (aligned with existing golden files), and validation assertions use Chainsaw to check CTRF report structure instead of weak grep patterns.

Closes #498

Summary

Strengthen UAT assert-recipe.yaml and validation assertions to verify expected component names, constraints, and deployment order instead of just checking componentRefs count > 0 and grepping for CTRF.

Motivation / Context

UAT assertions were trivially passing — a recipe with a single wrong component would pass, and || true on validate steps masked real failures. As new cloud/accelerator variants are added, every new UAT suite inherits these gaps.

Fixes: #498
Related: N/A

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • Build/CI/tooling

Component(s) Affected

  • CLI (cmd/aicr, pkg/cli)
  • API server (cmd/aicrd, pkg/api, pkg/server)
  • Recipe engine / data (pkg/recipe)
  • Bundlers (pkg/bundler, pkg/component/*)
  • Collectors / snapshotter (pkg/collector, pkg/snapshotter)
  • Validator (pkg/validator)
  • Core libraries (pkg/errors, pkg/k8s)
  • Docs/examples (docs/, examples/)
  • Other: UAT Chainsaw tests (tests/uat/)

Implementation Notes

Recipe assertions (4 files rewritten):

  • Replaced (length(componentRefs) > 0): true with full structural match: componentRefs (every component by name, alphabetically sorted), constraints, and deploymentOrder
  • EKS files aligned to existing golden files (tests/chainsaw/cli/cuj1-training/ and tests/chainsaw/ai-conformance/offline/)
  • AKS files generated from actual aicr recipe output — 11 training / 14 inference components including prometheus-adapter from the cross-cutting monitoring-hpa overlay

Validation assertions (4 files rewritten + 4 new):

  • Rewrote assert-validate-multiphase.yaml with Chainsaw-compatible assertions: structural match for failed: 0, pending: 0, other: 0; JMESPath for (results.summary.tests > 0) and (results.summary.skipped == results.summary.tests) (the --no-cluster contract)
  • Added symmetric assert-validate-deployment.yaml files (same assertions)
  • Note: Chainsaw v0.2.14 has a known issue with == `0` JMESPath comparisons, so zero-value fields use direct structural match instead

Chainsaw test flow (4 files updated):

  • Removed || true from both validate-deployment and validate-multiphase steps — failures are now surfaced instead of silently swallowed
  • Replaced weak grep-based assertion steps with python3 JSON wrapper (injects apiVersion/kind for Chainsaw) + chainsaw assert against the assertion files
  • Follows existing codebase pattern from tests/chainsaw/cli/bundle-scheduling/ which prepends synthetic headers for non-Kubernetes resources

Testing

# Recipe assertions verified against generated recipes
aicr recipe --service eks --accelerator h100 --intent training --os ubuntu --platform kubeflow -o /tmp/r.yaml
chainsaw assert --resource /tmp/r.yaml --file tests/uat/aws/tests/cuj1-training/assert-recipe.yaml
# Repeated for all 4 CUJs — all pass

# CTRF validation assertion verified end-to-end
aicr validate --recipe /tmp/r.yaml --snapshot mock-snapshot.yaml --phase deployment --no-cluster -o /tmp/v.json
# python3 wrapper + chainsaw assert — passes

# yamllint clean on all tests/uat/ files
yamllint tests/uat/

Risk Assessment

  • Low — Isolated change, well-tested, easy to revert
  • Medium — Touches multiple components or has broader impact
  • High — Breaking change, affects critical paths, or complex rollout

Rollout notes: YAML-only changes to UAT test assertions. No Go code, no runtime behavior changes. UAT environments need python3 on PATH (standard on GitHub-hosted runners). If overlay changes add/remove components, the corresponding assert-recipe.yaml must be updated — this is intentional to catch stale UAT tests.

Checklist

  • Tests pass locally (make test with -race)
  • Linter passes (make lint) — yamllint clean; Go lint failures are pre-existing in tests/chainsaw/ai-conformance/main.go
  • I did not skip/disable tests to make CI green
  • I added/updated tests for new functionality
  • I updated docs if user-facing behavior changed — N/A, no user-facing changes
  • Changes follow existing patterns in the codebase
  • Commits are cryptographically signed (git commit -S)

@yuanchen8911
Copy link
Copy Markdown
Contributor

/lgtm

Copy link
Copy Markdown
Contributor

@yuanchen8911 yuanchen8911 left a comment

Choose a reason for hiding this comment

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

Cross-reviewed by Claude Code, Codex, and CodeRabbit — all 3 independently verified component lists, constraints, and deployment orders against overlay chains for all 4 CUJs. CTRF assertion structure validated. No findings.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 10, 2026

Coverage Report ✅

Metric Value
Coverage 74.5%
Threshold 70%
Status Pass
Coverage Badge
![Coverage](https://img.shields.io/badge/coverage-74.5%25-green)

No Go source files changed in this PR.

Replace trivially-passing UAT assertions with full structural checks.
Recipe assertions now verify every expected component by name (aligned
with existing golden files), and validation assertions use Chainsaw to
check CTRF report structure instead of weak grep patterns.

Closes #498

Made-with: Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: UAT assert-recipe.yaml tests are too weak — assert expected component names, not just count > 0

2 participants