From 1d66a49742e96f3724c0ac32d27407b2b24df7ed Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 03:57:11 +0300 Subject: [PATCH] perf(cli)!: write compact JSON through a streaming encoder --- README.md | 6 +- cmd/morphic/command_test.go | 2 +- cmd/morphic/compile.go | 118 ++++++++++++++++---------- cmd/morphic/compile_test.go | 93 ++++++++++++++++++++ cmd/morphic/edgecases_test.go | 99 ++++++++++++++------- cmd/morphic/help_test.go | 9 +- cmd/morphic/testdata/compile-help.txt | 5 +- 7 files changed, 255 insertions(+), 77 deletions(-) diff --git a/README.md b/README.md index ccb1d64c..345dc549 100644 --- a/README.md +++ b/README.md @@ -93,7 +93,8 @@ go build -o morphic ./cmd/morphic ### CLI `morphic compile` lowers one OpenAPI 3.x spec into Morphic IR JSON on stdout, and writes -diagnostics to stderr. +diagnostics to stderr. Stdout is indented for reading; a file written with `-o` is compact, +which is about half the bytes, unless `--pretty` asks for the indented form. ```bash morphic compile openapi.yaml # IR JSON to stdout @@ -114,7 +115,8 @@ The flags below are `compile`'s: | Flag | Meaning | |---|---| -| `-o ` | Write IR JSON to `` instead of stdout. | +| `-o ` | Write IR JSON to `` instead of stdout, compact rather than indented. | +| `--pretty` | Indent the JSON `-o` writes; stdout is indented either way. | | `--fail-on error\|warning` | Exit non-zero when a diagnostic at or above this severity is emitted (default `error`). | | `--skip-validate` | Skip the referential-integrity `validate` pass. | | `--explain ` | Report what compiling produced at this source coordinate instead of writing the document. | diff --git a/cmd/morphic/command_test.go b/cmd/morphic/command_test.go index a94b45b4..7e9beaa0 100644 --- a/cmd/morphic/command_test.go +++ b/cmd/morphic/command_test.go @@ -65,7 +65,7 @@ func TestNewCompileFlags_DefinesEveryFlag(t *testing.T) { // compileFlagNames is every flag compile accepts, asserted from both the // constructor and the command-table entry so the two cannot drift. -var compileFlagNames = []string{"o", "fail-on", "skip-validate", "explain"} +var compileFlagNames = []string{"o", "fail-on", "skip-validate", "explain", "pretty"} func TestCommand_PrintFlagsDocumentsTheCommandsOwnFlags(t *testing.T) { t.Parallel() diff --git a/cmd/morphic/compile.go b/cmd/morphic/compile.go index bad5c41f..a36971e3 100644 --- a/cmd/morphic/compile.go +++ b/cmd/morphic/compile.go @@ -70,7 +70,8 @@ func newCompileCommand() command { summary: "lower an API spec (OpenAPI 3.x) into Morphic IR JSON", usage: "morphic compile [flags]", description: "Lower an API spec (OpenAPI 3.x) into Morphic IR JSON on stdout, and write\n" + - "diagnostics to stderr.\n\n" + + "diagnostics to stderr. Output to stdout is indented for reading; a file\n" + + "written with -o is compact unless --pretty asks for the indented form.\n\n" + "--explain reports what compiling produced at one source coordinate — the\n" + "type node interned there, the coordinates interned beneath it, and the\n" + "diagnostics stamped at it — instead of writing the document.", @@ -89,6 +90,7 @@ type compileOptions struct { failOn string skipValidate bool explain string + pretty bool } // newCompileFlags returns compile's FlagSet and the options its flags write @@ -108,6 +110,8 @@ func newCompileFlags() (*flag.FlagSet, *compileOptions) { "skip the referential-integrity validate pass") fs.StringVar(&opts.explain, "explain", "", "report what compiling produced at this source pointer instead of writing IR JSON") + fs.BoolVar(&opts.pretty, "pretty", false, + "indent the IR JSON -o writes; stdout is indented either way") return fs, &opts } @@ -171,7 +175,7 @@ func compileSpec(specPath string, opts compileOptions, stdout, stderr io.Writer) explainDocument(stdout, res.Document, res.Diagnostics, opts.explain) return exitCodeFor(res.Diagnostics, opts.failOn) } - if err := writeCompiled(opts.outPath, stdout, res.Document); err != nil { + if err := writeCompiled(opts, stdout, res.Document); err != nil { emitf(stderr, "morphic: %v\n", err) return 2 } @@ -251,26 +255,30 @@ func severityRank(s ir.Severity) int { } } -// writeCompiled emits doc's pretty IR JSON to outPath, or to stdout when outPath -// is empty. For a file destination, doc is marshalled in full before any file is -// touched, so a failed marshal never disturbs a file already there — see -// marshalDocument — and the bytes are then published atomically by replaceFile. -func writeCompiled(outPath string, stdout io.Writer, doc *ir.Document) error { - if outPath == "" { - return writeDocument(stdout, doc) - } - raw, err := marshalDocument(doc) - if err != nil { - return err +// writeCompiled emits doc's IR JSON to opts.outPath, or to stdout when it is +// empty. Stdout is indented because a person is reading it; a file is compact, +// which is about half the bytes, unless --pretty asks for the indented form. +// +// A file destination is encoded straight into replaceFile's temp file rather +// than into a slice handed over afterwards. That drops a whole copy of the +// output and keeps the property the marshal-first order used to provide: a +// document that will not marshal removes the temp file and leaves whatever is +// at outPath untouched, because outPath is only ever reached by the rename. +func writeCompiled(opts compileOptions, stdout io.Writer, doc *ir.Document) error { + if opts.outPath == "" { + return encodeDocument(stdout, doc, true) } - return replaceFile(outPath, raw) + return replaceFile(opts.outPath, func(w io.Writer) error { + return encodeDocument(w, doc, opts.pretty) + }) } -// replaceFile writes raw to outPath atomically: the bytes land in a temp file in -// the destination's own directory — so the publishing rename never crosses a -// filesystem boundary — and replace outPath only once all of them are on disk. -// A failed or partial write therefore leaves outPath's previous content intact -// instead of truncating it. +// replaceFile writes what fill produces to outPath atomically: fill's bytes land +// in a temp file in the destination's own directory — so the publishing rename +// never crosses a filesystem boundary — and replace outPath only once all of +// them are on disk. A failed or partial write therefore leaves outPath's +// previous content intact instead of truncating it, and so does a fill that +// fails halfway through. // // Publishing by rename replaces the directory entry rather than the bytes behind // it, which is what makes the swap atomic and which costs four things a @@ -302,13 +310,13 @@ func writeCompiled(outPath string, stdout io.Writer, doc *ir.Document) error { // property this function exists to provide; making the swap itself survive a // crash would need an fsync on the parent directory and is deliberately out of // scope. -func replaceFile(outPath string, raw []byte) error { +func replaceFile(outPath string, fill func(io.Writer) error) error { perm, replacing, err := destMode(outPath) if err != nil { return err } - tmp, err := writeTemp(outPath, raw) + tmp, err := writeTemp(outPath, fill) if err != nil { return err } @@ -343,11 +351,11 @@ func destMode(outPath string) (os.FileMode, bool, error) { return info.Mode().Perm(), true, nil } -// writeTemp writes raw to a newly created file beside outPath and returns that +// writeTemp runs fill into a newly created file beside outPath and returns that // file's path. Creation is O_EXCL under a random name, so it neither clobbers an // existing file nor follows a symlink planted at the name it drew; that makes the // unpredictability of the suffix a convenience, not a security boundary. -func writeTemp(outPath string, raw []byte) (string, error) { +func writeTemp(outPath string, fill func(io.Writer) error) (string, error) { dir := filepath.Dir(outPath) base := filepath.Base(outPath) @@ -360,7 +368,7 @@ func writeTemp(outPath string, raw []byte) (string, error) { if err != nil { return "", fmt.Errorf("create output %q: %w", outPath, err) } - if err := fillTemp(f, tmp, raw); err != nil { + if err := fillTemp(f, tmp, fill); err != nil { return "", err } return tmp, nil @@ -369,17 +377,17 @@ func writeTemp(outPath string, raw []byte) (string, error) { outPath, maxTempAttempts) } -// fillTemp writes raw to f, flushes it to the filesystem, and closes it, +// fillTemp runs fill into f, flushes it to the filesystem, and closes it, // removing tmp if any step fails so a failed run leaves no debris beside the -// destination. +// destination — including a fill that fails after writing part of its output. // // The Sync is what lets replaceFile's caller believe the rename publishes // durable bytes: without it the rename can expose a file whose contents are // still only in the page cache, and a crash before writeback leaves the // destination short or empty — the same loss writing through a temp file exists // to prevent. -func fillTemp(f outputFile, tmp string, raw []byte) error { - if err := writeRaw(f, raw); err != nil { +func fillTemp(f outputFile, tmp string, fill func(io.Writer) error) error { + if err := fill(f); err != nil { _ = f.Close() _ = os.Remove(tmp) return err @@ -396,26 +404,50 @@ func fillTemp(f outputFile, tmp string, raw []byte) error { return nil } -// marshalDocument renders doc to indented JSON with a trailing newline (the -// same bytes as irtest.WriteGolden). It is factored out of writeDocument so -// writeCompiled can marshal a file destination fully in memory before it ever -// opens — and so truncates — outPath. -func marshalDocument(doc *ir.Document) ([]byte, error) { - raw, err := json.MarshalIndent(doc, "", " ") +// destWriter passes writes through to w and records the error w returned. It is +// what keeps a destination that refused the bytes distinguishable from a +// document that would not marshal: json.Encoder reports both as one error from +// Encode, and they are different things to tell a user about. +type destWriter struct { + w io.Writer + err error +} + +func (d *destWriter) Write(p []byte) (int, error) { + n, err := d.w.Write(p) if err != nil { - return nil, fmt.Errorf("marshal ir document: %w", err) + d.err = err } - return append(raw, '\n'), nil + return n, err } -// writeDocument marshals doc to indented JSON with a trailing newline (the same -// bytes as irtest.WriteGolden) and writes it to w. -func writeDocument(w io.Writer, doc *ir.Document) error { - raw, err := marshalDocument(doc) - if err != nil { - return err +// encodeDocument writes doc's IR JSON to w, ending in a newline either way. +// +// The two forms take different routes on purpose, and neither is the obvious +// choice for the other. Indented output goes through json.MarshalIndent — the +// same bytes irtest.WriteGolden writes — because a json.Encoder with SetIndent +// indents into a second buffer it keeps for itself, which costs roughly twice +// the allocation for an identical result. Compact output goes through the +// encoder precisely because there is no second pass: it hands the marshalled +// bytes to w directly, where json.Marshal would first copy them into a slice +// for the caller to write. +func encodeDocument(w io.Writer, doc *ir.Document, indent bool) error { + if indent { + raw, err := json.MarshalIndent(doc, "", " ") + if err != nil { + return fmt.Errorf("marshal ir document: %w", err) + } + return writeRaw(w, append(raw, '\n')) } - return writeRaw(w, raw) + + dest := &destWriter{w: w} + if err := json.NewEncoder(dest).Encode(doc); err != nil { + if dest.err != nil { + return fmt.Errorf("write ir document: %w", dest.err) + } + return fmt.Errorf("marshal ir document: %w", err) + } + return nil } // writeRaw writes raw to w, wrapping any error with context. diff --git a/cmd/morphic/compile_test.go b/cmd/morphic/compile_test.go index 0ec71571..dc823d91 100644 --- a/cmd/morphic/compile_test.go +++ b/cmd/morphic/compile_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -108,3 +109,95 @@ func TestRun_UsageErrors(t *testing.T) { }) } } + +// unsortedSpec declares its schemas out of alphabetical order and its paths in +// an order of their own, so a document compiled from it exercises both halves +// of the determinism invariant: maps emitted in sorted-key order and slices in +// source order. +const unsortedSpec = `openapi: 3.1.0 +info: {title: Ordered, version: "1"} +paths: + /zebra: + get: {operationId: getZebra, responses: {"200": {description: ok}}} + /apple: + get: {operationId: getApple, responses: {"200": {description: ok}}} +components: + schemas: + Zeta: {type: object, properties: {b: {type: string}, a: {type: integer}}} + Alpha: {type: string} + Mid: {type: array, items: {type: string}} +` + +// compileToFile runs compile over spec with the given extra flags and returns +// the bytes -o wrote. +func compileToFile(t *testing.T, spec string, flags ...string) []byte { + t.Helper() + out := filepath.Join(t.TempDir(), "ir.json") + args := append([]string{"compile", spec, "-o", out}, flags...) + var stdout, stderr bytes.Buffer + + require.Equal(t, 0, run(args, &stdout, &stderr), "stderr: %s", stderr.String()) + + raw, err := os.ReadFile(out) + require.NoError(t, err) + return raw +} + +// TestRun_FileOutputIsCompact pins the artifact format: -o writes compact JSON, +// which is one line and a trailing newline, since a raw newline cannot appear +// inside a JSON string. +func TestRun_FileOutputIsCompact(t *testing.T) { + t.Parallel() + spec := writeFile(t, "spec.yaml", testspec.Tiny) + + raw := compileToFile(t, spec) + + assert.Equal(t, 1, bytes.Count(raw, []byte("\n")), + "compact JSON is one line plus its trailing newline, got:\n%s", raw) + var doc ir.Document + require.NoError(t, json.Unmarshal(raw, &doc)) + assert.Equal(t, "Tiny", doc.Name) +} + +// TestRun_PrettyRestoresIndentedFile pins the escape hatch, and pins it against +// the bytes that were there before rather than against "looks indented": -o +// -pretty must write exactly what stdout writes, which is the format -o itself +// used to write. +func TestRun_PrettyRestoresIndentedFile(t *testing.T) { + t.Parallel() + spec := writeFile(t, "spec.yaml", testspec.Tiny) + + raw := compileToFile(t, spec, "-pretty") + + var stdout, stderr bytes.Buffer + require.Equal(t, 0, run([]string{"compile", spec}, &stdout, &stderr), "stderr: %s", stderr.String()) + assert.Empty(t, cmp.Diff(stdout.String(), string(raw)), + "-pretty must write the bytes stdout gets") +} + +// TestRun_CompactOutputKeepsDocumentOrder is the determinism check the format +// change has to survive: compacting the indented artifact must reproduce the +// compact one byte for byte. Whitespace removal cannot reorder anything, so the +// two agreeing means the encoder emitted the same map keys in the same sorted +// order and the same slices in the same source order as the marshaller it +// replaced. Compiling twice pins that a second run agrees with the first. +func TestRun_CompactOutputKeepsDocumentOrder(t *testing.T) { + t.Parallel() + spec := writeFile(t, "spec.yaml", unsortedSpec) + + compact := compileToFile(t, spec) + pretty := compileToFile(t, spec, "-pretty") + + var flattened bytes.Buffer + require.NoError(t, json.Compact(&flattened, pretty)) + assert.Empty(t, cmp.Diff(flattened.String()+"\n", string(compact)), + "compacting the indented artifact must reproduce the compact one exactly") + + assert.Empty(t, cmp.Diff(string(compact), string(compileToFile(t, spec))), + "two runs over one spec must write the same bytes") + + // The assertion above is only worth something if the spec really does force + // a reordering, so confirm the emitted key order is not the declared one. + assert.Less(t, bytes.Index(compact, []byte("Alpha")), bytes.Index(compact, []byte("Zeta")), + "schemas must be emitted in sorted-key order, not declaration order") +} diff --git a/cmd/morphic/edgecases_test.go b/cmd/morphic/edgecases_test.go index 3e3c36d7..cb16613e 100644 --- a/cmd/morphic/edgecases_test.go +++ b/cmd/morphic/edgecases_test.go @@ -269,24 +269,45 @@ func TestRenderDiagnostics_WithAndWithoutSourcePath(t *testing.T) { assert.Contains(t, out, "warning ir/dangling type:abc: no source file") } -func TestWriteDocument_MarshalError(t *testing.T) { +// TestEncodeDocument_ErrorPaths pins that both output forms tell the same two +// failures apart. The compact form is the one that needs saying: json.Encoder +// reports a refusing destination and an unmarshallable document as one error +// from Encode, so without the writer that records what the destination said, +// every disk failure would be reported as a marshal failure. +func TestEncodeDocument_ErrorPaths(t *testing.T) { t.Parallel() - err := writeDocument(io.Discard, badDoc()) - require.Error(t, err) - assert.Contains(t, err.Error(), "marshal ir document") -} -func TestWriteDocument_WriteError(t *testing.T) { - t.Parallel() - err := writeDocument(failWriter{err: errors.New("disk gone")}, &ir.Document{Name: "ok"}) - require.Error(t, err) - assert.Contains(t, err.Error(), "write ir document") + tests := []struct { + name string + w io.Writer + doc *ir.Document + indent bool + want string + }{ + {"indented, bad document", io.Discard, badDoc(), true, "marshal ir document"}, + {"compact, bad document", io.Discard, badDoc(), false, "marshal ir document"}, + {"indented, refusing destination", + failWriter{err: errors.New("disk gone")}, &ir.Document{Name: "ok"}, true, "write ir document"}, + {"compact, refusing destination", + failWriter{err: errors.New("disk gone")}, &ir.Document{Name: "ok"}, false, "write ir document"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := encodeDocument(tt.w, tt.doc, tt.indent) + + require.Error(t, err) + assert.Contains(t, err.Error(), tt.want) + }) + } } func TestWriteParsed_CreateError(t *testing.T) { t.Parallel() badOut := filepath.Join(t.TempDir(), "no-such-dir", "ir.json") - err := writeCompiled(badOut, io.Discard, &ir.Document{Name: "ok"}) + err := writeCompiled(compileOptions{outPath: badOut}, io.Discard, &ir.Document{Name: "ok"}) require.Error(t, err) assert.Contains(t, err.Error(), "create output") } @@ -305,7 +326,8 @@ func TestWriteParsed_WriteErrorClosesFile(t *testing.T) { // doc marshals fine; the write to the temp file is what fails, exercising // the close-and-return path. - err := writeCompiled(filepath.Join(t.TempDir(), "out.json"), io.Discard, &ir.Document{Name: "ok"}) + err := writeCompiled(compileOptions{outPath: filepath.Join(t.TempDir(), "out.json")}, + io.Discard, &ir.Document{Name: "ok"}) require.Error(t, err) assert.Contains(t, err.Error(), "write ir document") @@ -335,7 +357,7 @@ func TestWriteParsed_WriteErrorPreservesDestination(t *testing.T) { return &writeFailFile{File: f, writeErr: errors.New("disk full")}, nil }) - err := writeCompiled(out, io.Discard, &ir.Document{Name: "ok"}) + err := writeCompiled(compileOptions{outPath: out}, io.Discard, &ir.Document{Name: "ok"}) require.Error(t, err) assert.NotEqual(t, out, opened, "the write must go through a temp file, never the destination") @@ -344,27 +366,44 @@ func TestWriteParsed_WriteErrorPreservesDestination(t *testing.T) { assert.Equal(t, existing, string(got), "a failed write must not disturb the previous output") } +// TestWriteParsed_MarshalErrorLeavesFileUntouched pins both halves of encoding +// into the temp file instead of marshalling ahead of it: the temp file is +// created before the document is known to marshal, and a document that does not +// marshal must still leave nothing behind and leave the destination alone — +// outPath is only ever reached by the rename. func TestWriteParsed_MarshalErrorLeavesFileUntouched(t *testing.T) { - t.Parallel() - out := filepath.Join(t.TempDir(), "ir.json") + dir := t.TempDir() + out := filepath.Join(dir, "ir.json") const existing = `{"name":"Existing"}` + "\n" require.NoError(t, os.WriteFile(out, []byte(existing), 0o644)) - err := writeCompiled(out, io.Discard, badDoc()) + created := 0 + swapCreateOutput(t, func(path string, perm os.FileMode) (outputFile, error) { + created++ + return os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm) + }) + + err := writeCompiled(compileOptions{outPath: out}, io.Discard, badDoc()) require.Error(t, err) assert.Contains(t, err.Error(), "marshal ir document") + assert.Equal(t, 1, created, + "the document must be encoded into the temp file, not marshalled ahead of it") got, readErr := os.ReadFile(out) require.NoError(t, readErr) assert.Equal(t, existing, string(got), "a failed marshal must not truncate a file that already exists at outPath") + entries, readErr := os.ReadDir(dir) + require.NoError(t, readErr) + assert.Len(t, entries, 1, "a failed encode must leave no temp file beside the destination") } func TestWriteParsed_CloseError(t *testing.T) { wc := &closeFailWriteCloser{closeErr: errors.New("close failed")} swapCreateOutput(t, func(string, os.FileMode) (outputFile, error) { return wc, nil }) - err := writeCompiled(filepath.Join(t.TempDir(), "out.json"), io.Discard, &ir.Document{Name: "ok"}) + err := writeCompiled(compileOptions{outPath: filepath.Join(t.TempDir(), "out.json")}, + io.Discard, &ir.Document{Name: "ok"}) require.Error(t, err) assert.Contains(t, err.Error(), "close output") @@ -380,7 +419,7 @@ func TestWriteParsed_ReplacesAtomically(t *testing.T) { out := filepath.Join(dir, "ir.json") require.NoError(t, os.WriteFile(out, []byte("stale\n"), 0o640)) - require.NoError(t, writeCompiled(out, io.Discard, &ir.Document{Name: "Fresh"})) + require.NoError(t, writeCompiled(compileOptions{outPath: out}, io.Discard, &ir.Document{Name: "Fresh"})) got, err := os.ReadFile(out) require.NoError(t, err) @@ -418,7 +457,7 @@ func TestWriteParsed_ReadOnlyDirFails(t *testing.T) { // Restore write permission so t.TempDir's own cleanup can remove the tree. t.Cleanup(func() { _ = os.Chmod(dir, 0o755) }) - err := writeCompiled(out, io.Discard, &ir.Document{Name: "Fresh"}) + err := writeCompiled(compileOptions{outPath: out}, io.Discard, &ir.Document{Name: "Fresh"}) require.Error(t, err) assert.Contains(t, err.Error(), "create output") @@ -439,7 +478,7 @@ func TestWriteParsed_SymlinkIsReplaced(t *testing.T) { require.NoError(t, os.WriteFile(target, []byte("PREVIOUS\n"), 0o644)) require.NoError(t, os.Symlink(target, link)) - require.NoError(t, writeCompiled(link, io.Discard, &ir.Document{Name: "Fresh"})) + require.NoError(t, writeCompiled(compileOptions{outPath: link}, io.Discard, &ir.Document{Name: "Fresh"})) info, err := os.Lstat(link) require.NoError(t, err) @@ -466,7 +505,7 @@ func TestWriteParsed_HardLinkIsBroken(t *testing.T) { require.NoError(t, os.WriteFile(out, []byte("PREVIOUS\n"), 0o644)) require.NoError(t, os.Link(out, other)) - require.NoError(t, writeCompiled(out, io.Discard, &ir.Document{Name: "Fresh"})) + require.NoError(t, writeCompiled(compileOptions{outPath: out}, io.Discard, &ir.Document{Name: "Fresh"})) got, err := os.ReadFile(out) require.NoError(t, err) @@ -503,7 +542,7 @@ func TestWriteParsed_NearLimitNameFails(t *testing.T) { require.NoError(t, os.WriteFile(tooLong, []byte("PREVIOUS\n"), 0o644), "precondition: a truncating write to this name succeeds") - err := writeCompiled(tooLong, io.Discard, &ir.Document{Name: "Fresh"}) + err := writeCompiled(compileOptions{outPath: tooLong}, io.Discard, &ir.Document{Name: "Fresh"}) require.Error(t, err, "the temp name must not fit where the destination does") assert.Contains(t, err.Error(), "create output") @@ -520,7 +559,8 @@ func TestWriteParsed_StatError(t *testing.T) { notDir := filepath.Join(t.TempDir(), "file") require.NoError(t, os.WriteFile(notDir, []byte("x"), 0o644)) - err := writeCompiled(filepath.Join(notDir, "ir.json"), io.Discard, &ir.Document{Name: "ok"}) + err := writeCompiled(compileOptions{outPath: filepath.Join(notDir, "ir.json")}, + io.Discard, &ir.Document{Name: "ok"}) require.Error(t, err) assert.Contains(t, err.Error(), "stat output") @@ -537,7 +577,7 @@ func TestWriteParsed_TempNameCollisionRetries(t *testing.T) { }) out := filepath.Join(t.TempDir(), "ir.json") - require.NoError(t, writeCompiled(out, io.Discard, &ir.Document{Name: "ok"})) + require.NoError(t, writeCompiled(compileOptions{outPath: out}, io.Discard, &ir.Document{Name: "ok"})) require.Len(t, names, 2, "a taken temp name must be retried, not reused") assert.NotEqual(t, names[0], names[1], "each attempt must draw a fresh name") @@ -550,7 +590,8 @@ func TestWriteParsed_TempNamesExhausted(t *testing.T) { return nil, os.ErrExist }) - err := writeCompiled(filepath.Join(t.TempDir(), "ir.json"), io.Discard, &ir.Document{Name: "ok"}) + err := writeCompiled(compileOptions{outPath: filepath.Join(t.TempDir(), "ir.json")}, + io.Discard, &ir.Document{Name: "ok"}) require.Error(t, err) assert.Contains(t, err.Error(), "no unused temp name") @@ -570,7 +611,7 @@ func TestWriteParsed_SyncErrorPreservesDestination(t *testing.T) { f := &syncFailWriteCloser{syncErr: errors.New("sync refused")} swapCreateOutput(t, func(string, os.FileMode) (outputFile, error) { return f, nil }) - err := writeCompiled(out, io.Discard, &ir.Document{Name: "ok"}) + err := writeCompiled(compileOptions{outPath: out}, io.Discard, &ir.Document{Name: "ok"}) require.Error(t, err) assert.Contains(t, err.Error(), "sync output") @@ -591,7 +632,7 @@ func TestWriteParsed_ChmodError(t *testing.T) { t.Cleanup(func() { chmodOutput = origChmod }) chmodOutput = func(string, os.FileMode) error { return errors.New("chmod refused") } - err := writeCompiled(out, io.Discard, &ir.Document{Name: "ok"}) + err := writeCompiled(compileOptions{outPath: out}, io.Discard, &ir.Document{Name: "ok"}) require.Error(t, err) assert.Contains(t, err.Error(), "chmod output") @@ -608,7 +649,7 @@ func TestWriteParsed_RenameError(t *testing.T) { t.Cleanup(func() { renameOutput = origRename }) renameOutput = func(string, string) error { return errors.New("rename refused") } - err := writeCompiled(out, io.Discard, &ir.Document{Name: "ok"}) + err := writeCompiled(compileOptions{outPath: out}, io.Discard, &ir.Document{Name: "ok"}) require.Error(t, err) assert.Contains(t, err.Error(), "replace output") @@ -620,6 +661,6 @@ func TestWriteParsed_RenameError(t *testing.T) { func TestWriteParsed_ToStdoutSuccess(t *testing.T) { t.Parallel() var buf bytes.Buffer - require.NoError(t, writeCompiled("", &buf, &ir.Document{Name: "ok"})) + require.NoError(t, writeCompiled(compileOptions{}, &buf, &ir.Document{Name: "ok"})) assert.True(t, bytes.HasSuffix(buf.Bytes(), []byte("\n"))) } diff --git a/cmd/morphic/help_test.go b/cmd/morphic/help_test.go index 5fda7ead..37d1e4f7 100644 --- a/cmd/morphic/help_test.go +++ b/cmd/morphic/help_test.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "encoding/json" "os" "path/filepath" "testing" @@ -11,6 +12,7 @@ import ( "github.com/stretchr/testify/require" "github.com/dexpace/morphic/internal/testspec" + "github.com/dexpace/morphic/ir" "github.com/dexpace/morphic/ir/irtest" ) @@ -76,7 +78,12 @@ func TestRun_HelpFlagAsFlagValue(t *testing.T) { "--help must be consumed as -o's value, not treated as a help request") raw, err := os.ReadFile(filepath.Join(dir, "--help")) require.NoError(t, err) - assert.Contains(t, string(raw), `"name": "Tiny"`) + // Read the name back out of the document rather than matching the artifact's + // text: what this test is about is which file was written, not how it is + // formatted, and -o's formatting is a flag away from changing. + var doc ir.Document + require.NoError(t, json.Unmarshal(raw, &doc)) + assert.Equal(t, "Tiny", doc.Name) } func TestRun_HelpFormsAgree(t *testing.T) { diff --git a/cmd/morphic/testdata/compile-help.txt b/cmd/morphic/testdata/compile-help.txt index 115d6b19..58b417db 100644 --- a/cmd/morphic/testdata/compile-help.txt +++ b/cmd/morphic/testdata/compile-help.txt @@ -2,7 +2,8 @@ usage: morphic compile [flags] Lower an API spec (OpenAPI 3.x) into Morphic IR JSON on stdout, and write -diagnostics to stderr. +diagnostics to stderr. Output to stdout is indented for reading; a file +written with -o is compact unless --pretty asks for the indented form. --explain reports what compiling produced at one source coordinate — the type node interned there, the coordinates interned beneath it, and the @@ -15,5 +16,7 @@ flags: fail (exit 1) on diagnostics at or above this severity: error|warning (default "error") -o string write IR JSON to this file instead of stdout + -pretty + indent the IR JSON -o writes; stdout is indented either way -skip-validate skip the referential-integrity validate pass