diff --git a/CLAUDE.md b/CLAUDE.md index 8ab793b3..d45c08d0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -233,7 +233,7 @@ When drafting Slack messages, PR descriptions, review replies, release notes, or # Testing - Prefer integration tests to cover most cases. Use unit tests when integration tests are not practical. -- In-house test helpers (no external assertion/snapshot deps): `internal/must` for fatal generic assertions (`must.Eq` renders go-cmp diffs; `must.Nil`/`NotNil` handle typed nils), and `internal/snap` for file-snapshot tests. `snap.Match(t, s)` stores one file per snapshot under `__snapshots__/` next to the test; missing snapshots are created locally but fail in CI; `UPDATE_SNAPS=true go test ./...` rewrites them. A package using snapshots must wire `func TestMain(m *testing.M) { os.Exit(snap.Clean(m)) }` — obsolete snapshots then fail the run (or are deleted under `UPDATE_SNAPS=true`); cleanup is skipped on filtered (`-run`/`-skip`) or failed runs. Sanitize volatile values before matching with label-anchored regexes, not value-shaped ones (see `sanitizeSnapshot` in `internal/output`; RE2 has no lookahead, so a bare semver pattern corrupts IPv4 strings). +- In-house test helpers (no external assertion/snapshot deps): `internal/must` for fatal generic assertions (`must.Eq` renders go-cmp diffs; `must.Nil`/`NotNil` handle typed nils), and `internal/snap` for file-snapshot tests. `snap.Match(t, s)` stores one file per snapshot under `__snapshots__/` next to the test; missing snapshots are created locally but fail in CI; `UPDATE_SNAPS=true go test ./...` rewrites them. `snap.MatchJSON(t, raw, "data.currentVersion", ...)` snapshots a JSON document in canonical pretty-printed form, masking the values at the given dotted paths with `` (a path that doesn't resolve fails the test; objects only, no array indexing). A package using snapshots must wire `func TestMain(m *testing.M) { os.Exit(snap.Clean(m)) }` — obsolete snapshots then fail the run (or are deleted under `UPDATE_SNAPS=true`); cleanup is skipped on filtered (`-run`/`-skip`) or failed runs. Sanitize volatile values before matching with label-anchored regexes, not value-shaped ones (see `sanitizeSnapshot` in `internal/output`; RE2 has no lookahead, so a bare semver pattern corrupts IPv4 strings). - **When fixing a bug, always add an integration test** that fails before the fix and passes after. This prevents regressions and documents the exact scenario that was broken. - Integration tests that run the CLI binary with Bubble Tea must use a PTY (`github.com/creack/pty`) since Bubble Tea requires a terminal. Use `pty.Start(cmd)` instead of `cmd.CombinedOutput()`, read output with `io.Copy()`, and send keystrokes by writing to the PTY (e.g., `ptmx.Write([]byte("\r"))` for Enter). - Mark every integration test with `t.Parallel()` unless it shares external state with other tests. Today the main blocker is the Docker daemon: tests that start LocalStack containers cannot run concurrently because lstk's container discovery matches by `(image, internal port)`, so two parallel runs would cross-contaminate. Tests that only touch the filesystem, mock servers, or the CLI binary itself should be parallel. diff --git a/internal/output/__snapshots__/TestEnvelopeSink_UpdateCheckedEnvelopeJSON_1.snap b/internal/output/__snapshots__/TestEnvelopeSink_UpdateCheckedEnvelopeJSON_1.snap new file mode 100644 index 00000000..18cb6cb4 --- /dev/null +++ b/internal/output/__snapshots__/TestEnvelopeSink_UpdateCheckedEnvelopeJSON_1.snap @@ -0,0 +1,12 @@ +{ + "command": "update", + "data": { + "currentVersion": "", + "latestVersion": "", + "updateAvailable": true + }, + "error": null, + "schemaVersion": 1, + "status": "ok", + "warnings": [] +} diff --git a/internal/output/envelope_sink_test.go b/internal/output/envelope_sink_test.go index 93138776..5e475728 100644 --- a/internal/output/envelope_sink_test.go +++ b/internal/output/envelope_sink_test.go @@ -1,10 +1,12 @@ package output import ( + "encoding/json" "errors" "testing" "github.com/localstack/lstk/internal/must" + "github.com/localstack/lstk/internal/snap" ) func TestEnvelopeSink_SuccessWithNoEvents(t *testing.T) { @@ -87,6 +89,19 @@ func TestEnvelopeSink_UpdateCheckedEvent(t *testing.T) { } } +// TestEnvelopeSink_UpdateCheckedEnvelopeJSON pins the full serialized +// envelope shape (schemaVersion, status, data, warnings) as a snapshot, +// masking the version fields that vary in real runs. +func TestEnvelopeSink_UpdateCheckedEnvelopeJSON(t *testing.T) { + sink := NewEnvelopeSink(FormatJSON) + sink.Emit(UpdateCheckedEvent{CurrentVersion: "2.2.1", LatestVersion: "2.3.0", Available: true}) + + envelope := sink.Result("update", nil) + raw, err := json.Marshal(envelope) + must.NoError(t, err) + snap.MatchJSON(t, raw, "data.currentVersion", "data.latestVersion") +} + func TestEnvelopeSink_UpdateAppliedEvent(t *testing.T) { t.Parallel() diff --git a/internal/snap/snap.go b/internal/snap/snap.go index 66af85d9..fdee4d14 100644 --- a/internal/snap/snap.go +++ b/internal/snap/snap.go @@ -14,6 +14,8 @@ package snap import ( + "bytes" + "encoding/json" "errors" "flag" "fmt" @@ -44,14 +46,78 @@ var unsafeChars = regexp.MustCompile(`[^A-Za-z0-9._-]+`) // UPDATE_SNAPS=true. func Match(t testing.TB, got string) { t.Helper() - if n := flagValue("test.count"); n != "" && n != "1" { - t.Fatalf("snap: -count > 1 is not supported (snapshot call numbering would repeat)") + match(t, got, callerSnapshotDir(t)) +} + +// MatchJSON snapshots got (a JSON document) in canonical pretty-printed form +// (two-space indent, object keys sorted by encoding/json), after replacing +// the values at the given dotted paths (e.g. "data.currentVersion") with the +// placeholder "". Use the paths to mask values that legitimately change +// between runs. A path that doesn't resolve fails the test, so a masked +// field disappearing is caught rather than silently ignored. Paths traverse +// JSON objects only; there is no array-index syntax. +func MatchJSON(t testing.TB, got []byte, maskPaths ...string) { + t.Helper() + // The returns after Fatalf matter: tests exercise this path with a fake + // TB whose Fatalf does not stop the goroutine like *testing.T's does, + // and a fallthrough would store a bogus snapshot. + var v any + if err := json.Unmarshal(got, &v); err != nil { + t.Fatalf("snap: value is not valid JSON: %v", err) + return } - _, file, _, ok := runtime.Caller(1) + for _, path := range maskPaths { + if !mask(v, strings.Split(path, ".")) { + t.Fatalf("snap: mask path %q not found in JSON", path) + return + } + } + // An Encoder rather than MarshalIndent so the "" placeholder isn't + // HTML-escaped (u003c/u003e) in the stored snapshot. Encode appends the + // trailing newline. + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetEscapeHTML(false) + enc.SetIndent("", " ") + if err := enc.Encode(v); err != nil { + t.Fatalf("snap: %v", err) + } + match(t, buf.String(), callerSnapshotDir(t)) +} + +// mask walks nested JSON objects along path segments and replaces the final +// value with "", reporting whether the full path resolved. +func mask(v any, segs []string) bool { + m, ok := v.(map[string]any) + if !ok { + return false + } + if len(segs) == 1 { + if _, ok := m[segs[0]]; !ok { + return false + } + m[segs[0]] = "" + return true + } + return mask(m[segs[0]], segs[1:]) +} + +// callerSnapshotDir resolves the __snapshots__ directory next to the test +// file that called the exported Match/MatchJSON function (two frames up). +func callerSnapshotDir(t testing.TB) string { + t.Helper() + _, file, _, ok := runtime.Caller(2) if !ok { t.Fatal("snap: cannot resolve calling test file") } - dir := filepath.Join(filepath.Dir(file), "__snapshots__") + return filepath.Join(filepath.Dir(file), "__snapshots__") +} + +func match(t testing.TB, got, dir string) { + t.Helper() + if n := flagValue("test.count"); n != "" && n != "1" { + t.Fatalf("snap: -count > 1 is not supported (snapshot call numbering would repeat)") + } mu.Lock() key := dir + "|" + t.Name() diff --git a/internal/snap/snap_test.go b/internal/snap/snap_test.go index bccbb06f..0f6e9c6b 100644 --- a/internal/snap/snap_test.go +++ b/internal/snap/snap_test.go @@ -174,6 +174,50 @@ func TestMatchCountsCallsWithinOneTest(t *testing.T) { } } +func TestMatchJSONMasksAndCanonicalizes(t *testing.T) { + t.Setenv("CI", "") + t.Setenv("UPDATE_SNAPS", "") + path := snapPath(t, "TestFakeJSON", "1") + + ft := &fakeT{name: "TestFakeJSON"} + MatchJSON(ft, []byte(`{"zebra":1,"data":{"version":"4.14.1","name":"aws"}}`), "data.version") + if ft.failed { + t.Fatalf("unexpected failure: %q", ft.msg) + } + content, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + got := string(content) + if !strings.Contains(got, `"version": ""`) { + t.Fatalf("masked value missing: %s", got) + } + if strings.Contains(got, "4.14.1") { + t.Fatalf("volatile value leaked into snapshot: %s", got) + } + if strings.Index(got, `"data"`) > strings.Index(got, `"zebra"`) { + t.Fatalf("keys not sorted: %s", got) + } +} + +func TestMatchJSONFailsOnMissingMaskPath(t *testing.T) { + t.Setenv("CI", "") + ft := &fakeT{name: "TestFakeJSONBadPath"} + MatchJSON(ft, []byte(`{"data":{}}`), "data.version") + if !ft.fatal { + t.Fatal("missing mask path must be fatal") + } +} + +func TestMatchJSONFailsOnInvalidJSON(t *testing.T) { + t.Setenv("CI", "") + ft := &fakeT{name: "TestFakeJSONInvalid"} + MatchJSON(ft, []byte(`not json`)) + if !ft.fatal { + t.Fatal("invalid JSON must be fatal") + } +} + func TestReportObsolete(t *testing.T) { dir := t.TempDir() live := filepath.Join(dir, "TestLive_1.snap") @@ -209,3 +253,17 @@ func TestReportObsolete(t *testing.T) { } } } + +func TestMatchJSONFatalPathsWriteNoSnapshot(t *testing.T) { + t.Setenv("CI", "") + dir := filepath.Join(filepath.Dir(callerFile(t)), "__snapshots__") + + MatchJSON(&fakeT{name: "TestFakeJSONInvalid"}, []byte(`not json`)) + MatchJSON(&fakeT{name: "TestFakeJSONBadPath"}, []byte(`{"data":{}}`), "data.version") + + for _, name := range []string{"TestFakeJSONInvalid_1.snap", "TestFakeJSONBadPath_1.snap"} { + if _, err := os.Stat(filepath.Join(dir, name)); !os.IsNotExist(err) { + t.Fatalf("fatal MatchJSON call must not write %s", name) + } + } +}