Skip to content

Commit 0ba8067

Browse files
committed
test(bdd): prototype portable live smoke composition
Add a target-driven live suite with explicit provider and smoke phases, bounded retry and compensation primitives, and a portable cluster-maintenance example. Relates to #1186 Signed-off-by: Stephanie Baum <sbaum@nvidia.com>
1 parent bd89567 commit 0ba8067

27 files changed

Lines changed: 1903 additions & 48 deletions

tests/bdd/AGENTS.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ so `nvcf-cli` and the NVCF API remain the product-validation boundary.
4242
`Suite.Runner`.
4343
- `godog_test.go` owns the live entry points and the fake-runner
4444
wiring tests.
45+
- `live/` owns the separate portable runner. It may reuse `harness`, `dsl`,
46+
and `steps`, but must not add target modes or feature selection to
47+
`godog_test.go`.
4548

4649
A step handler that does anything beyond argv assembly, ledger
4750
snapshot, runner invocation, and result capture is a smell. Move the
@@ -113,6 +116,9 @@ logic into `dsl/`.
113116
`tools/ncp-local-cluster/Makefile` and
114117
`deploy/stacks/self-managed/Makefile` are intentionally maintained
115118
so an operator can clean by hand without involving `go test`.
119+
- The portable `live/` suite rejects `BDD_CLEANUP_MODE`. Those cleanup
120+
commands are specific to ncp-local. Remote and production targets must never
121+
inherit local cleanup behavior.
116122
- Cleanup belongs in `harness/cleanup.go`, never in `steps/`. Do not
117123
introduce a `Given the cluster is freshly destroyed` Given or
118124
similar; the conflict precheck inside every feature Background is
@@ -223,10 +229,37 @@ multi-cluster feature:
223229
check that a destructive command was issued. Do not deep-equality
224230
the recorder; consolidating equivalent steps in the future must not
225231
break these tests.
232+
- Portable wiring tests live in `live/` and use the same fake
233+
`CommandRunner`. Portable features run one file per Godog phase. The runner
234+
restores step-exported environment values and ledger-backed files between
235+
files so one selectable smoke cannot become an undocumented provider for the
236+
next.
226237
- Live entry points (`TestSingleClusterUp`, `TestMultiClusterUp`,
227238
`TestSingleClusterHelmfile`) skip under `-short`. They build the
228239
CLI and exercise real `make`/`kubectl`/`helm` against k3d.
229240

241+
## Portable live features
242+
243+
- Target YAML is versioned, non-secret data. It supplies execution coordinates
244+
and workload inputs only. It must not select features or contain destructive
245+
consent.
246+
- Features are selected by the operator independently of the target. One
247+
optional provider runs before one or more smoke features. Do not rely on
248+
Godog file ordering.
249+
- Every portable smoke is independently selectable. It declares required
250+
target fields through environment preconditions and does not depend on an
251+
earlier feature's exported variables or files.
252+
- Every Kubernetes command exposes both kubeconfig and context in Gherkin.
253+
Ambient current context is not portable and is unsafe for remote targets.
254+
- Cluster-wide mutations require separate invocation-time consent and use the
255+
product's expected-cluster identity guard before mutation.
256+
- Register compensation before its destructive command. Keep the action,
257+
target, and timeout visible. Compensation handlers hide only reverse-order
258+
execution and continue-after-failure mechanics.
259+
- Do not add capability auto-detection, target-based feature skipping, or
260+
product validation to the runner. Missing inputs fail the feature's visible
261+
preconditions; unsupported behavior fails through the real CLI or API.
262+
230263
## Style
231264

232265
- Plain ASCII only in committed text. No bold, no em dashes, no

tests/bdd/PLAN.md

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,8 @@ refactor in every consumer; that is a feature.
119119
| `When I run command:` (docstring) | Multi-line form for commands that don't fit on one line. Same recording semantics. |
120120
| `When I successfully run command {string}` | Single-line command that must exit 0. Preserves the command result for later output assertions and records the resolved command in the successful-command cache. |
121121
| `When I successfully run command:` (docstring) | Multi-line form for commands that must exit 0. Uses the same result recording, interpolation, logging, and cache semantics as the single-line form. |
122+
| `And after this scenario I successfully run command within {string}:` (docstring) | Registers the visible command as bounded scenario compensation. Compensations run in reverse registration order after success or failure, continue past failures, and preserve product behavior in the command text. |
123+
| `Then within {string} this command should succeed, checking every {string}:` (docstring) | Repeats the visible command after product-level nonzero exits until it exits 0 or the visible timeout expires. Runner failures stop immediately. The successful result remains available to output assertions. |
122124
| `When I run command with a terminal:` (docstring) | Same as the docstring form, but stdin is attached to a pseudo-terminal so the child sees a TTY on fd 0. For commands that gate interactive-only behavior on a TTY, such as `nvcf-cli self-hosted up` (its auth-gate mints the admin token only when stdin is a terminal). No input is written; stdout and stderr are captured separately as usual. |
123125
| `When I export command output to environment variable {string}` | Exports the previous command's trimmed stdout under the named env var. Fails the step unless the prior command exited 0 and produced non-empty stdout. Snapshotted by the env Ledger; restored at suite teardown. |
124126

@@ -158,6 +160,7 @@ original order. Repeated options and empty values are preserved.
158160
| `Then the command exit code should be {int}` | Last-run exit code. |
159161
| `Then the command output should contain {string}` | Substring match on combined stdout + stderr. |
160162
| `Then the command output should not contain {string}` | Negative substring match. |
163+
| `Then the JSON command output should contain:` (JSON docstring) | Parses the last command's stdout and the expected docstring as JSON, then requires the typed expected object fields while allowing additional actual fields. Lists remain order- and length-sensitive. |
161164
| `Then file {string} should exist` | |
162165
| `Then yaml file {string} key {string} should equal {string}` | Reads the YAML file, walks the dotted key path, compares to the value (with `${VAR}` expansion). |
163166
| `Then yaml file {string} key {string} should not be empty` | Same key resolution; passes if the resolved value is non-empty. Use for non-deterministic outputs (cluster IDs, identity sources) where exact-value assertions are wrong. |
@@ -206,10 +209,18 @@ them via `${VAR}` in command strings and table cells.
206209
| `REPO_ROOT` | `git rev-parse --show-toplevel` at suite start | Absolute path to the repo root. Required when invoking `make -C deploy/stacks/self-managed` because the Makefile's `-C` changes cwd; relative paths to fixtures from there break. |
207210
| `NGC_API_KEY` / `SAMPLE_NGC_ORG` / `SAMPLE_NGC_TEAM` | The operator's shell | Passed through unchanged. An environment-variable precondition step asserts they are non-empty before any scenario uses them. |
208211

212+
The portable live runner additionally maps its selected target to stable
213+
`BDD_NVCF_*`, `BDD_COMPUTE_*`, `BDD_NVCA_*`, and `BDD_WORKLOAD_*` variables.
214+
It runs each selected feature separately and restores step-exported environment
215+
variables and ledger-backed file changes between features. CLI state and the
216+
successful bootstrap command cache remain suite-scoped. See `live/README.md`
217+
for the target schema and safety contract.
218+
209219
Feature files may also export their own env vars at runtime via
210220
`When I export command output to environment variable {string}`. Those
211-
are snapshotted by the env Ledger and restored at teardown so they do
212-
not leak into later test binaries in the same `go test` invocation.
221+
are snapshotted by the env Ledger and restored at teardown. The portable live
222+
runner restores them after each selected feature so they cannot become an
223+
implicit input to another smoke.
213224
The EKS Helmfile feature exports `EKS_GATEWAY_ADDR` this way from
214225
`kubectl get gateway` after the Gateway resource is Programmed.
215226

@@ -255,8 +266,9 @@ restoration ledger:
255266
body, mode) in memory. Repeat writes against the same path during a
256267
suite do not overwrite the snapshot; only the first write records.
257268
- At suite teardown, the runner restores every registered path to its
258-
pre-suite state. Files that did not exist before are deleted; files
259-
that did are rewritten with the original bytes and mode.
269+
pre-suite state. The portable live runner performs the same restoration
270+
between selected features. Files that did not exist before are deleted;
271+
files that did are rewritten with the original bytes and mode.
260272
- `Config.LedgerDir` (`out/<run-id>/originals/`) is reserved for an
261273
on-disk variant if very large fixtures ever push memory limits.
262274
Today the directory is created but unused.
@@ -344,13 +356,16 @@ old `tests/bdd` tree.
344356
tests/bdd/
345357
features/ (Gherkin, already committed)
346358
fixtures/ (sample env + CLI config, already committed)
359+
targets/ (non-secret portable live coordinates)
347360
STEPS.md (this document)
348361
349362
harness/ (phase 1)
350363
config.go (paths, env exports)
351364
runner.go (CommandRunner around infra.Runner)
352365
ledger.go (file restoration ledger)
353366
cache.go (command-success cache)
367+
deferred.go (bounded scenario compensation stack)
368+
eventually.go (bounded command retry)
354369
suite.go (lifecycle: build CLI, set env, teardown)
355370
356371
dsl/ (phase 1)
@@ -365,6 +380,8 @@ tests/bdd/
365380
assertion_steps.go (exit code, output contains, file/yaml/json)
366381
infra_steps.go (cluster bootstrap, image pull secret)
367382
383+
live/ (portable target loader and phased runner)
384+
368385
godog_test.go (phase 3 and 4: TestSingleClusterUp,
369386
TestMultiClusterUp, TestSingleClusterHelmfile,
370387
wiring tests with fakes)
@@ -432,14 +449,17 @@ func (c *CommandCache) Has(commandText string) bool
432449
// Suite is the top-level lifecycle owner. Built once per go test
433450
// invocation; runs one or more feature files.
434451
type Suite struct {
435-
Config Config
436-
Runner CommandRunner
437-
Ledger *Ledger
438-
Cache *CommandCache
452+
Config Config
453+
Runner CommandRunner
454+
Ledger *Ledger
455+
EnvLedger *EnvLedger
456+
Cache *CommandCache
439457
}
440458

441459
func NewSuite(t *testing.T) (*Suite, error) // builds nvcf-cli, sets NVCF_CLI + REPO_ROOT
442-
func (s *Suite) Teardown() error // calls Ledger.RestoreAll
460+
func NewSuiteWithOptions(t *testing.T, options SuiteOptions) (*Suite, error)
461+
func (s *Suite) RestoreFeatureState() error // restores step-owned files and env vars
462+
func (s *Suite) Teardown() error // also restores the exact CLI state file
443463
```
444464

445465
#### dsl package

tests/bdd/README.md

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ in `AGENTS.md`.
1212
```
1313
features/ Gherkin feature files: single-cluster CLI, multi-cluster CLI,
1414
single/multi-cluster Helmfile (k3d), and single/multi-cluster
15-
EKS Helmfile (non-local).
15+
EKS Helmfile (non-local), plus target-neutral smoke features.
1616
fixtures/ Starting environment YAML and CLI config the features copy from.
1717
harness/ Suite lifecycle: Config, CommandRunner, Ledger, CommandCache,
1818
Suite. Builds nvcf-cli at suite start; exports NVCF_CLI and
@@ -22,6 +22,9 @@ dsl/ Pure helpers: ${VAR} interpolation, dotted-path YAML upsert
2222
row matching, kubectl manifest builders.
2323
steps/ Godog step handlers. Every handler is a thin wrapper around a
2424
dsl helper or CommandRunner.Run; no domain validation.
25+
live/ Separate portable runner for optional provider plus independently
26+
selectable smoke features. See live/README.md.
27+
targets/ Versioned, non-secret execution coordinates for portable smokes.
2528
godog_test.go
2629
Live entry points (TestSingleClusterUp, TestSingleClusterUpOneClick,
2730
TestMultiClusterUp, TestSingleClusterHelmfile, TestMultiClusterHelmfile,
@@ -44,9 +47,14 @@ cd tests/bdd
4447
go test -short ./...
4548
```
4649

47-
Live runs build nvcf-cli, bring up a real k3d cluster, and exercise the
48-
feature end to end. They require an NGC API key and sample registry
49-
coordinates.
50+
To run one smoke feature against an already installed local or remote target,
51+
or to compose a provider with several target-neutral smokes, see
52+
`live/README.md`.
53+
54+
The install-suite live runs build nvcf-cli, bring up a real k3d cluster, and
55+
exercise the feature end to end. They require an NGC API key and sample
56+
registry coordinates. The portable live runner attaches to the target selected
57+
in its target file.
5058

5159
Each `-run` argument is anchored with `^...$` so the live entry point
5260
runs without also matching its `...FeatureFileWiresToSteps` wiring

tests/bdd/dsl/interp.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,11 @@ limitations under the License.
2121
package dsl
2222

2323
import (
24+
"fmt"
2425
"os"
2526
"regexp"
27+
"sort"
28+
"strings"
2629
)
2730

2831
// braced matches the only environment variable form the DSL recognizes:
@@ -41,3 +44,27 @@ func Interpolate(s string) string {
4144
return os.Getenv(name)
4245
})
4346
}
47+
48+
// InterpolateRequired expands the same syntax as Interpolate but rejects an
49+
// unset or empty referenced variable. Use it for lifecycle coordinates where
50+
// an empty expansion could point a mutation or restoration at the wrong path.
51+
func InterpolateRequired(s string) (string, error) {
52+
missingSet := make(map[string]struct{})
53+
resolved := braced.ReplaceAllStringFunc(s, func(match string) string {
54+
name := match[2 : len(match)-1]
55+
value := os.Getenv(name)
56+
if value == "" {
57+
missingSet[name] = struct{}{}
58+
}
59+
return value
60+
})
61+
if len(missingSet) == 0 {
62+
return resolved, nil
63+
}
64+
missing := make([]string, 0, len(missingSet))
65+
for name := range missingSet {
66+
missing = append(missing, name)
67+
}
68+
sort.Strings(missing)
69+
return "", fmt.Errorf("required interpolation variables are empty: %s", strings.Join(missing, ", "))
70+
}

tests/bdd/dsl/interp_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,17 @@ func TestInterpolateBracedOnly(t *testing.T) {
4646
})
4747
}
4848
}
49+
50+
func TestInterpolateRequired(t *testing.T) {
51+
t.Setenv("BDD_REQUIRED", "value")
52+
got, err := InterpolateRequired("prefix-${BDD_REQUIRED}")
53+
if err != nil {
54+
t.Fatalf("interpolate: %v", err)
55+
}
56+
if got != "prefix-value" {
57+
t.Fatalf("got %q", got)
58+
}
59+
if _, err := InterpolateRequired("${BDD_REQUIRED_MISSING}/state"); err == nil {
60+
t.Fatal("expected missing variable error")
61+
}
62+
}

tests/bdd/dsl/jsoncmp.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,21 @@ import (
2222
"fmt"
2323
)
2424

25+
// MatchJSONDocument compares an expected JSON document to command output.
26+
// MatchSubset requires every expected object field while allowing additional
27+
// actual fields. Mismatch errors identify paths without printing values.
28+
func MatchJSONDocument(actualJSON, expectedJSON string, mode MatchMode) error {
29+
var expected any
30+
if err := json.Unmarshal([]byte(Interpolate(expectedJSON)), &expected); err != nil {
31+
return fmt.Errorf("parse expected json: invalid JSON")
32+
}
33+
var actual any
34+
if err := json.Unmarshal([]byte(actualJSON), &actual); err != nil {
35+
return fmt.Errorf("parse command output: invalid JSON")
36+
}
37+
return deepCompare(expected, actual, mode, "")
38+
}
39+
2540
// JSONContainsRows parses raw as a JSON array of objects and, for each
2641
// row map, asserts that an object matching every (key, value) pair in
2742
// the row exists in the array. Extra objects in the array are

tests/bdd/dsl/jsoncmp_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,26 @@ const helmListOutput = `[
2525
{"name": "api", "namespace": "nvcf", "status": "deployed"}
2626
]`
2727

28+
func TestMatchJSONDocumentContainsTypedSubset(t *testing.T) {
29+
actual := `{"mode":"CordonAndDrain","configChanged":true,"count":0,"extra":"ok"}`
30+
expected := `{"mode":"CordonAndDrain","configChanged":true,"count":0}`
31+
if err := MatchJSONDocument(actual, expected, MatchSubset); err != nil {
32+
t.Fatalf("match: %v", err)
33+
}
34+
}
35+
36+
func TestMatchJSONDocumentRejectsMismatchAndInvalidJSON(t *testing.T) {
37+
if err := MatchJSONDocument(`{"configChanged":false}`, `{"configChanged":true}`, MatchSubset); err == nil {
38+
t.Fatal("expected typed value mismatch")
39+
}
40+
if err := MatchJSONDocument("not json", `{}`, MatchSubset); err == nil {
41+
t.Fatal("expected actual JSON parse error")
42+
}
43+
if err := MatchJSONDocument(`{}`, "not json", MatchSubset); err == nil {
44+
t.Fatal("expected expected JSON parse error")
45+
}
46+
}
47+
2848
func TestJSONContainsRowsMatchesExpected(t *testing.T) {
2949
rows := []map[string]string{
3050
{"name": "nats", "namespace": "nats-system", "status": "deployed"},

0 commit comments

Comments
 (0)