diff --git a/commands/apply.go b/commands/apply.go index 905cbef..085ac64 100644 --- a/commands/apply.go +++ b/commands/apply.go @@ -122,7 +122,7 @@ func (c *ApplyCommand) AutocompleteFlags() complete.Flags { c.Meta.AutocompleteFlags(command.FlagSetClient), complete.Flags{ "--tasks": taskFileAutocomplete(), - "--tasks-format": tasksFormatAutocomplete(), + "--tasks-format": recipeFormatAutocomplete(), "--verbose": complete.PredictNothing, "--json": complete.PredictNothing, "--host": complete.PredictAnything, @@ -173,7 +173,7 @@ func (c *ApplyCommand) Run(args []string) int { resolvedHost := resolveSshFlags(c.host, c.sudo, c.acceptNewHostKeys) - formatOverride, err := parseTasksFormatFlag(c.tasksFormatFlag) + formatOverride, err := parseRecipeFormatFlag("--tasks-format", c.tasksFormatFlag) if err != nil { c.Ui.Error(err.Error()) return 1 diff --git a/commands/export.go b/commands/export.go index f29c748..6f39510 100644 --- a/commands/export.go +++ b/commands/export.go @@ -25,6 +25,10 @@ type ExportCommand struct { output string varsOutput string + // formatFlag is the raw --format value; it is normalised by + // parseRecipeFormatFlag in Run and then governs both the recipe and + // the companion vars-file, overriding either output's extension. + formatFlag string overwrite bool redact bool apps []string @@ -52,6 +56,7 @@ func (c *ExportCommand) Examples() map[string]string { "Export the local server to tasks.yml + tasks.vars.yml": fmt.Sprintf("%s %s", appName, c.Name()), "Export a remote server over SSH": fmt.Sprintf("%s %s --host deploy@dokku.example.com", appName, c.Name()), "Stream a self-contained recipe to stdout": fmt.Sprintf("%s %s --output -", appName, c.Name()), + "Stream a JSON5 recipe to stdout": fmt.Sprintf("%s %s --output - --format json5", appName, c.Name()), "Redact secrets into a fill-in-the-blanks vars-file": fmt.Sprintf("%s %s --redact", appName, c.Name()), "Export only a single app": fmt.Sprintf("%s %s --app my-app", appName, c.Name()), } @@ -71,8 +76,9 @@ func (c *ExportCommand) ParsedArguments(args []string) (map[string]command.Argum func (c *ExportCommand) FlagSet() *flag.FlagSet { f := c.Meta.FlagSet(c.Name(), command.FlagSetClient) - f.StringVar(&c.output, "output", "tasks.yml", "path to write the recipe to; pass - to stream a self-contained recipe to stdout") - f.StringVar(&c.varsOutput, "vars-output", "", "path to write the companion vars-file to (defaults to .vars.)") + f.StringVar(&c.output, "output", defaultRecipeOutput, "path to write the recipe to; pass - to stream a self-contained recipe to stdout") + f.StringVar(&c.formatFlag, "format", "", "write the recipe and vars-file as this format (yaml or json5) instead of inferring it from the --output extension. Without an explicit --output, json5 writes "+defaultRecipeOutputJSON5+"; this is also the only way to get JSON5 on stdout.") + f.StringVar(&c.varsOutput, "vars-output", "", "path to write the companion vars-file to (defaults to .vars.; --format overrides its format)") f.BoolVar(&c.overwrite, "overwrite", false, "overwrite existing output files without prompting") f.BoolVar(&c.redact, "redact", false, "write placeholder values into the vars-file instead of real secrets") f.StringArrayVar(&c.apps, "app", nil, "restrict the export to the named app (repeatable)") @@ -87,6 +93,7 @@ func (c *ExportCommand) AutocompleteFlags() complete.Flags { c.Meta.AutocompleteFlags(command.FlagSetClient), complete.Flags{ "--output": taskFileAutocomplete(), + "--format": recipeFormatAutocomplete(), "--vars-output": taskFileAutocomplete(), "--overwrite": complete.PredictNothing, "--redact": complete.PredictNothing, @@ -114,12 +121,31 @@ func (c *ExportCommand) Run(args []string) int { return 1 } + formatOverride, err := parseRecipeFormatFlag("--format", c.formatFlag) + if err != nil { + c.Ui.Error(err.Error()) + return 1 + } + + // Resolve the write target up front. --format json5 with no explicit + // --output moves the default to tasks.json (and, through + // deriveVarsOutput, tasks.vars.json), and every later use of c.output + // - the overwrite prompt, the write, the summary, the Next steps line + // - has to agree on one path. flags.Changed is only meaningful after + // flags.Parse. Validating here also means a typo'd --format fails + // before an SSH control master is opened or the server is read. + var recipeFormat string + c.output, recipeFormat = resolveRecipeOutput(c.output, formatOverride, flags.Changed("output")) + if msg := recipeOutputFormatMismatch(c.output, formatOverride); msg != "" { + c.Ui.Warn(msg) + } + resolvedHost := resolveSshFlags(c.host, c.sudo, c.acceptNewHostKeys) if resolvedHost != "" { defer subprocess.CloseSshControlMaster(resolvedHost) } - toStdout := c.output == "-" + toStdout := c.output == taskFileStdin res, err := tasks.ExportRecipe(tasks.ExportOptions{ Apps: c.apps, @@ -147,10 +173,6 @@ func (c *ExportCommand) Run(args []string) int { return 1 } - recipeFormat := taskFileFormatYAML - if !toStdout { - recipeFormat = detectTaskFileFormat(c.output) - } recipeBytes, err := res.MarshalRecipe(recipeFormat) if err != nil { c.Ui.Error(fmt.Sprintf("marshal recipe: %v", err)) @@ -169,6 +191,14 @@ func (c *ExportCommand) Run(args []string) int { if varsOutput == "" { varsOutput = deriveVarsOutput(c.output) } + // --format governs the pair: when it is given, the vars-file matches + // the recipe even if --vars-output names another extension. Without + // it the vars-file keeps following its own extension, so + // `--output tasks.yml --vars-output vars.json` still writes JSON. + varsFormat := formatOverride + if varsFormat == "" { + varsFormat = detectTaskFileFormat(varsOutput) + } writeVars := res.HasVars() // Overwrite check: both files are checked before either is written, so a @@ -204,7 +234,7 @@ func (c *ExportCommand) Run(args []string) int { return 1 } if writeVars { - varsBytes, err := res.MarshalVars(detectTaskFileFormat(varsOutput)) + varsBytes, err := res.MarshalVars(varsFormat) if err != nil { c.Ui.Error(fmt.Sprintf("marshal vars: %v", err)) return 1 diff --git a/commands/export_test.go b/commands/export_test.go index e6e3fae..b4092f7 100644 --- a/commands/export_test.go +++ b/commands/export_test.go @@ -168,6 +168,257 @@ func TestExportOutputValidates(t *testing.T) { } } +// TestExportOutputValidatesJSON5 is the JSON5 twin of +// TestExportOutputValidates: the pair --format json5 emits must round-trip +// through docket's own offline validation just as the YAML pair does. +func TestExportOutputValidatesJSON5(t *testing.T) { + defer subprocess.SetExecRunner(fakeExecRunner(exportCommandFixture()))() + + dir := t.TempDir() + t.Chdir(dir) + recipe := filepath.Join(dir, "tasks.json") + vars := filepath.Join(dir, "tasks.vars.json") + + c, ui := newExportCommand() + if code := c.Run([]string{"--format", "json5"}); code != 0 { + t.Fatalf("export exit = %d: %s", code, ui.ErrorWriter.String()) + } + + valArgs := []string{"--tasks", recipe, "--vars-file", vars, "--strict"} + oldArgs := os.Args + os.Args = append([]string{"docket", "validate"}, valArgs...) + defer func() { os.Args = oldArgs }() + + vui := cli.NewMockUi() + v := &ValidateCommand{Meta: command.Meta{Ui: vui}} + if code := v.Run(valArgs); code != 0 { + rb, _ := os.ReadFile(recipe) + vb, _ := os.ReadFile(vars) + t.Fatalf("docket validate --strict exit = %d, want 0\n--- validate stderr ---\n%s\n--- recipe ---\n%s\n--- vars ---\n%s", + code, vui.ErrorWriter.String(), rb, vb) + } +} + +// TestExportCommandFormatJSON5WritesJSONPair pins the default-path swap +// for the pair of files export writes: tasks.json plus tasks.vars.json, +// and no .yml left behind. +func TestExportCommandFormatJSON5WritesJSONPair(t *testing.T) { + defer subprocess.SetExecRunner(fakeExecRunner(exportCommandFixture()))() + + dir := t.TempDir() + t.Chdir(dir) + + c, ui := newExportCommand() + if code := c.Run([]string{"--format", "json5"}); code != 0 { + t.Fatalf("Run exit = %d, want 0: %s", code, ui.ErrorWriter.String()) + } + + for _, name := range []string{"tasks.yml", "tasks.vars.yml"} { + if _, err := os.Stat(filepath.Join(dir, name)); err == nil { + t.Errorf("--format json5 should not write %s", name) + } + } + + recipe, err := os.ReadFile(filepath.Join(dir, "tasks.json")) + if err != nil { + t.Fatalf("tasks.json not written: %v", err) + } + if !strings.HasPrefix(string(recipe), "[") { + t.Errorf("recipe should open with a JSON5 array:\n%s", recipe) + } + if !strings.Contains(string(recipe), "{{ .web_API_KEY }}") { + t.Errorf("recipe should reference the input:\n%s", recipe) + } + + vars, err := os.ReadFile(filepath.Join(dir, "tasks.vars.json")) + if err != nil { + t.Fatalf("tasks.vars.json not written: %v", err) + } + if !strings.HasPrefix(string(vars), "{") { + t.Errorf("vars-file should be a JSON object:\n%s", vars) + } + if !strings.Contains(string(vars), `"web_API_KEY"`) || !strings.Contains(string(vars), "abc123") { + t.Errorf("vars-file should hold the real value under the input name:\n%s", vars) + } + // export always emits per-task "cannot read this back" warnings, so + // assert on the absence of the mismatch one specifically. + if warn := ui.ErrorWriter.String(); strings.Contains(warn, "does not match") { + t.Errorf("matching extensions should not warn about a mismatch:\n%s", warn) + } +} + +// TestExportCommandFormatOverridesOutputExtension pins that --format wins +// over an explicit --output extension, and that the resulting file - whose +// name now lies about its contents - is warned about. +func TestExportCommandFormatOverridesOutputExtension(t *testing.T) { + defer subprocess.SetExecRunner(fakeExecRunner(exportCommandFixture()))() + + dir := t.TempDir() + recipe := filepath.Join(dir, "tasks.yml") + + c, ui := newExportCommand() + if code := c.Run([]string{"--output", recipe, "--format", "json5"}); code != 0 { + t.Fatalf("Run exit = %d, want 0: %s", code, ui.ErrorWriter.String()) + } + + body, err := os.ReadFile(recipe) + if err != nil { + t.Fatalf("recipe not written: %v", err) + } + if !strings.HasPrefix(string(body), "[") { + t.Errorf("--format json5 should have won over the .yml extension:\n%s", body) + } + if warn := ui.ErrorWriter.String(); !strings.Contains(warn, "--tasks-format json5") { + t.Errorf("a lying extension should warn how to read it back:\n%s", warn) + } +} + +// TestExportCommandFormatGovernsVarsFile pins decision 3 of #410: an +// explicit --format sets the vars-file format too, even when +// --vars-output names a different extension. +func TestExportCommandFormatGovernsVarsFile(t *testing.T) { + defer subprocess.SetExecRunner(fakeExecRunner(exportCommandFixture()))() + + dir := t.TempDir() + recipe := filepath.Join(dir, "tasks.json5") + vars := filepath.Join(dir, "tasks.vars.yml") + + c, ui := newExportCommand() + if code := c.Run([]string{"--output", recipe, "--vars-output", vars, "--format", "json5"}); code != 0 { + t.Fatalf("Run exit = %d, want 0: %s", code, ui.ErrorWriter.String()) + } + + body, err := os.ReadFile(vars) + if err != nil { + t.Fatalf("vars-file not written: %v", err) + } + // A quoted key is the JSON tell; the YAML encoder emits web_API_KEY: + // unquoted. JSON is valid YAML, so this file still loads under its + // .yml name - which is why only the recipe earns a warning. + if !strings.HasPrefix(string(body), "{") || !strings.Contains(string(body), `"web_API_KEY":`) { + t.Errorf("--format json5 should have won over the .yml vars extension:\n%s", body) + } +} + +// TestExportCommandVarsFileKeepsItsOwnExtensionWithoutFormat is the +// backwards-compatibility half: with no --format, the vars-file still +// follows its own --vars-output extension rather than the recipe's. +func TestExportCommandVarsFileKeepsItsOwnExtensionWithoutFormat(t *testing.T) { + tests := []struct { + name string + varsName string + wantPrefix string + }{ + {name: "json vars beside a yaml recipe", varsName: "vars.json", wantPrefix: "{"}, + {name: "yaml vars beside a yaml recipe", varsName: "vars.yml", wantPrefix: "web_API_KEY:"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defer subprocess.SetExecRunner(fakeExecRunner(exportCommandFixture()))() + + dir := t.TempDir() + vars := filepath.Join(dir, tt.varsName) + + c, ui := newExportCommand() + code := c.Run([]string{"--output", filepath.Join(dir, "tasks.yml"), "--vars-output", vars}) + if code != 0 { + t.Fatalf("Run exit = %d, want 0: %s", code, ui.ErrorWriter.String()) + } + + body, err := os.ReadFile(vars) + if err != nil { + t.Fatalf("vars-file not written: %v", err) + } + if !strings.HasPrefix(string(body), tt.wantPrefix) { + t.Errorf("vars-file should follow its own extension, want prefix %q:\n%s", tt.wantPrefix, body) + } + }) + } +} + +// TestExportCommandFormatJSON5ToStdout is the round trip #410 was filed +// for. Streaming still inlines values, so there is no vars-file and +// nothing lands on disk. +func TestExportCommandFormatJSON5ToStdout(t *testing.T) { + defer subprocess.SetExecRunner(fakeExecRunner(exportCommandFixture()))() + + dir := t.TempDir() + t.Chdir(dir) + + var ui *cli.MockUi + captured, exit := captureStdout(t, func() int { + var c *ExportCommand + c, ui = newExportCommand() + return c.Run([]string{"--output", "-", "--format", "json5"}) + }) + if exit != 0 { + t.Fatalf("exit = %d, want 0: %s", exit, ui.ErrorWriter.String()) + } + if !strings.HasPrefix(captured, "[") { + t.Errorf("stdout should open with a JSON5 array:\n%s", captured) + } + if !strings.Contains(captured, "abc123") { + t.Errorf("a streamed recipe inlines its values:\n%s", captured) + } + + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("read dir: %v", err) + } + if len(entries) != 0 { + t.Errorf("--output - should not write any file, found %d", len(entries)) + } +} + +// TestExportCommandFormatJSON5PromptsOnAdjustedPath is the export twin of +// init's --force sequencing test: the overwrite prompt has to name the +// path the swap produced. +func TestExportCommandFormatJSON5PromptsOnAdjustedPath(t *testing.T) { + defer subprocess.SetExecRunner(fakeExecRunner(exportCommandFixture()))() + + dir := t.TempDir() + t.Chdir(dir) + recipe := filepath.Join(dir, "tasks.json") + if err := os.WriteFile(recipe, []byte("OLD\n"), 0o644); err != nil { + t.Fatalf("seed tasks.json: %v", err) + } + + c, ui := newExportCommand() + ui.InputReader = strings.NewReader("n\n") + if code := c.Run([]string{"--format", "json5"}); code != 1 { + t.Fatalf("declined overwrite exit = %d, want 1", code) + } + if body, _ := os.ReadFile(recipe); string(body) != "OLD\n" { + t.Errorf("declined overwrite should leave the file alone: %q", body) + } + if out := ui.OutputWriter.String(); !strings.Contains(out, "tasks.json already exists") { + t.Errorf("prompt should name the adjusted path:\n%s", out) + } +} + +// TestExportCommandRejectsUnknownFormatBeforeReadingServer deliberately +// installs no fake exec runner: passing proves the --format value is +// rejected before tasks.ExportRecipe would have shelled out to dokku. +func TestExportCommandRejectsUnknownFormatBeforeReadingServer(t *testing.T) { + dir := t.TempDir() + recipe := filepath.Join(dir, "tasks.yml") + + c, ui := newExportCommand() + if code := c.Run([]string{"--format", "toml", "--output", recipe}); code != 1 { + t.Fatalf("exit = %d, want 1", code) + } + errOut := ui.ErrorWriter.String() + if !strings.Contains(errOut, "--format") { + t.Errorf("error should name --format:\n%s", errOut) + } + if !strings.Contains(errOut, "yaml, json5") { + t.Errorf("error should name the valid values:\n%s", errOut) + } + if _, err := os.Stat(recipe); err == nil { + t.Error("a rejected --format should not write the recipe") + } +} + func TestExportCommandSummaryExcludesGlobalPlay(t *testing.T) { // #345: one app plus a global play must report "(1 app)", not "(2 apps)". responses := exportCommandFixture() diff --git a/commands/fmt.go b/commands/fmt.go index df90aa3..98d9d23 100644 --- a/commands/fmt.go +++ b/commands/fmt.go @@ -91,7 +91,7 @@ func (c *FmtCommand) AutocompleteFlags() complete.Flags { "--check": complete.PredictNothing, "--diff": complete.PredictNothing, "--color": complete.PredictSet("auto", "always", "never"), - "--tasks-format": tasksFormatAutocomplete(), + "--tasks-format": recipeFormatAutocomplete(), }, ) } @@ -116,7 +116,7 @@ func (c *FmtCommand) Run(args []string) int { return 1 } - formatOverride, err := parseTasksFormatFlag(c.tasksFormatFlag) + formatOverride, err := parseRecipeFormatFlag("--tasks-format", c.tasksFormatFlag) if err != nil { c.Ui.Error(err.Error()) return 1 diff --git a/commands/init.go b/commands/init.go index d9e78b0..f4bb543 100644 --- a/commands/init.go +++ b/commands/init.go @@ -27,11 +27,15 @@ import ( type InitCommand struct { command.Meta - output string - name string - repo string - force bool - minimal bool + output string + // formatFlag is the raw --format value; it is normalised by + // parseRecipeFormatFlag in Run and then overrides whatever the + // --output extension would have implied. + formatFlag string + name string + repo string + force bool + minimal bool } func (c *InitCommand) Name() string { @@ -49,14 +53,15 @@ func (c *InitCommand) Help() string { func (c *InitCommand) Examples() map[string]string { appName := os.Getenv("CLI_APP_NAME") return map[string]string{ - "Scaffold tasks.yml using cwd defaults": fmt.Sprintf("%s %s", appName, c.Name()), - "Scaffold a JSON5 tasks.json instead": fmt.Sprintf("%s %s --output tasks.json", appName, c.Name()), - "Write a minimal one-task scaffold": fmt.Sprintf("%s %s --minimal", appName, c.Name()), - "Override the play and app name": fmt.Sprintf("%s %s --name web", appName, c.Name()), - "Override the git repository URL": fmt.Sprintf("%s %s --repo git@example.com:owner/repo.git", appName, c.Name()), - "Write to a specific path": fmt.Sprintf("%s %s --output path/to/tasks.yml", appName, c.Name()), - "Stream the rendered scaffold to stdout": fmt.Sprintf("%s %s --output -", appName, c.Name()), - "Overwrite an existing file": fmt.Sprintf("%s %s --force", appName, c.Name()), + "Scaffold tasks.yml using cwd defaults": fmt.Sprintf("%s %s", appName, c.Name()), + "Scaffold a JSON5 tasks.json instead": fmt.Sprintf("%s %s --format json5", appName, c.Name()), + "Write a minimal one-task scaffold": fmt.Sprintf("%s %s --minimal", appName, c.Name()), + "Override the play and app name": fmt.Sprintf("%s %s --name web", appName, c.Name()), + "Override the git repository URL": fmt.Sprintf("%s %s --repo git@example.com:owner/repo.git", appName, c.Name()), + "Write to a specific path": fmt.Sprintf("%s %s --output path/to/tasks.yml", appName, c.Name()), + "Stream the rendered scaffold to stdout": fmt.Sprintf("%s %s --output -", appName, c.Name()), + "Stream a JSON5 scaffold to stdout": fmt.Sprintf("%s %s --output - --format json5", appName, c.Name()), + "Overwrite an existing file": fmt.Sprintf("%s %s --force", appName, c.Name()), } } @@ -74,7 +79,8 @@ func (c *InitCommand) ParsedArguments(args []string) (map[string]command.Argumen func (c *InitCommand) FlagSet() *flag.FlagSet { f := c.Meta.FlagSet(c.Name(), command.FlagSetClient) - f.StringVar(&c.output, "output", "tasks.yml", "path to write the scaffold to; pass - to write to stdout") + f.StringVar(&c.output, "output", defaultRecipeOutput, "path to write the scaffold to; pass - to write to stdout") + f.StringVar(&c.formatFlag, "format", "", "write the scaffold as this format (yaml or json5) instead of inferring it from the --output extension. Without an explicit --output, json5 writes "+defaultRecipeOutputJSON5+"; this is also the only way to get JSON5 on stdout.") f.BoolVar(&c.force, "force", false, "overwrite an existing output file") f.BoolVar(&c.minimal, "minimal", false, "emit a minimal one-task scaffold without an inputs block") f.StringVar(&c.name, "name", defaultName(), "play name and default app input value") @@ -87,6 +93,7 @@ func (c *InitCommand) AutocompleteFlags() complete.Flags { c.Meta.AutocompleteFlags(command.FlagSetClient), complete.Flags{ "--output": taskFileAutocomplete(), + "--format": recipeFormatAutocomplete(), "--force": complete.PredictNothing, "--minimal": complete.PredictNothing, "--name": complete.PredictNothing, @@ -109,7 +116,24 @@ func (c *InitCommand) Run(args []string) int { return 1 } - toStdout := c.output == "-" + formatOverride, err := parseRecipeFormatFlag("--format", c.formatFlag) + if err != nil { + c.Ui.Error(err.Error()) + return 1 + } + + // Resolve the write target before anything else looks at it. With + // --format json5 and no explicit --output the default path becomes + // tasks.json, and the exists / --force check below has to stat that + // path, not the tasks.yml it would otherwise have defaulted to. + // flags.Changed is only meaningful after flags.Parse. + var format string + c.output, format = resolveRecipeOutput(c.output, formatOverride, flags.Changed("output")) + if msg := recipeOutputFormatMismatch(c.output, formatOverride); msg != "" { + c.Ui.Warn(msg) + } + + toStdout := c.output == taskFileStdin if !toStdout { if _, err := os.Stat(c.output); err == nil { @@ -123,14 +147,6 @@ func (c *InitCommand) Run(args []string) int { } } - // Format is inferred from the --output extension: tasks.json / - // tasks.json5 -> JSON5, anything else -> YAML. Stdout (--output -) - // has no extension to inspect, so it falls through to YAML. - format := tasks.FormatYAML - if !toStdout { - format = detectTaskFileFormat(c.output) - } - rendered, err := renderInit(initOptions{ Name: c.name, Repo: c.repo, diff --git a/commands/init_test.go b/commands/init_test.go index 99f64ba..d911c5c 100644 --- a/commands/init_test.go +++ b/commands/init_test.go @@ -513,26 +513,10 @@ func TestInitOutputDashWritesToStdout(t *testing.T) { dir := t.TempDir() t.Chdir(dir) - r, w, err := os.Pipe() - if err != nil { - t.Fatalf("pipe: %v", err) - } - origStdout := os.Stdout - os.Stdout = w - defer func() { os.Stdout = origStdout }() - - c := newTestInitCommand() - exitCh := make(chan int, 1) - go func() { - exitCh <- c.Run([]string{"--output", "-", "--name", "demo"}) - w.Close() - }() - - captured, err := readAllString(r) - if err != nil { - t.Fatalf("read pipe: %v", err) - } - if exit := <-exitCh; exit != 0 { + captured, exit := captureStdout(t, func() int { + return newTestInitCommand().Run([]string{"--output", "-", "--name", "demo"}) + }) + if exit != 0 { t.Errorf("exit = %d, want 0", exit) } @@ -550,15 +534,249 @@ func TestInitOutputDashWritesToStdout(t *testing.T) { } } +// TestInitFormatJSON5WritesTasksJSON covers the headline of #410: asking +// for JSON5 by name, with no --output, writes tasks.json rather than a +// JSON5 document under a .yml name. +func TestInitFormatJSON5WritesTasksJSON(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + + c, ui := newTestInitCommandUi() + if exit := c.Run([]string{"--format", "json5", "--name", "demo"}); exit != 0 { + t.Fatalf("exit = %d, want 0: %s", exit, ui.ErrorWriter.String()) + } + + if _, err := os.Stat(filepath.Join(dir, "tasks.yml")); err == nil { + t.Error("--format json5 should not write tasks.yml") + } + body, err := os.ReadFile(filepath.Join(dir, "tasks.json")) + if err != nil { + t.Fatalf("tasks.json not written: %v", err) + } + if !strings.HasPrefix(string(body), "[") { + t.Errorf("scaffold should open with a JSON5 array:\n%s", body) + } + if strings.HasPrefix(string(body), "---") { + t.Errorf("JSON5 scaffold should not carry the YAML document marker:\n%s", body) + } + if problems := tasks.Validate(body, tasks.ValidateOptions{Format: tasks.FormatNameJSON5}); len(problems) > 0 { + t.Errorf("scaffold did not validate: %+v", problems) + } + if out := ui.OutputWriter.String(); !strings.Contains(out, "Created tasks.json") { + t.Errorf("summary should name tasks.json:\n%s", out) + } + if warn := ui.ErrorWriter.String(); warn != "" { + t.Errorf("a matching extension should not warn:\n%s", warn) + } +} + +// TestInitFormatJSONAliasWritesTasksJSON pins the consequence of sharing +// one normaliser with --tasks-format: the json alias resolves to json5, +// so it drives the default-path swap too. +func TestInitFormatJSONAliasWritesTasksJSON(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + + if exit := newTestInitCommand().Run([]string{"--format", "json"}); exit != 0 { + t.Fatalf("exit = %d, want 0", exit) + } + if _, err := os.Stat(filepath.Join(dir, "tasks.json")); err != nil { + t.Errorf("--format json should write tasks.json: %v", err) + } +} + +// TestInitFormatYAMLKeepsDefaultPath guards the untouched default: only +// json5 moves the path. +func TestInitFormatYAMLKeepsDefaultPath(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + + if exit := newTestInitCommand().Run([]string{"--format", "yaml"}); exit != 0 { + t.Fatalf("exit = %d, want 0", exit) + } + if _, err := os.Stat(filepath.Join(dir, "tasks.json")); err == nil { + t.Error("--format yaml should not write tasks.json") + } + body, err := os.ReadFile(filepath.Join(dir, "tasks.yml")) + if err != nil { + t.Fatalf("tasks.yml not written: %v", err) + } + if !strings.HasPrefix(string(body), "---\n") { + t.Errorf("YAML scaffold should keep its document marker:\n%s", body) + } +} + +// TestInitExplicitOutputWinsOverFormatDefault pins the rule that a path +// the user typed is never rewritten, even when --format would otherwise +// have moved the default. The extension then disagrees with the bytes, +// which is legal and warned about. +func TestInitExplicitOutputWinsOverFormatDefault(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + + c, ui := newTestInitCommandUi() + if exit := c.Run([]string{"--output", "recipe.yml", "--format", "json5"}); exit != 0 { + t.Fatalf("exit = %d, want 0: %s", exit, ui.ErrorWriter.String()) + } + if _, err := os.Stat(filepath.Join(dir, "tasks.json")); err == nil { + t.Error("an explicit --output should not be replaced by the json5 default") + } + body, err := os.ReadFile(filepath.Join(dir, "recipe.yml")) + if err != nil { + t.Fatalf("recipe.yml not written: %v", err) + } + if !strings.HasPrefix(string(body), "[") { + t.Errorf("--format json5 should have won over the .yml extension:\n%s", body) + } + if warn := ui.ErrorWriter.String(); !strings.Contains(warn, "--tasks-format json5") { + t.Errorf("a lying extension should warn how to read it back:\n%s", warn) + } +} + +// TestInitFormatOverridesOutputExtension is the mirror case: --format +// yaml beats a .json extension. +func TestInitFormatOverridesOutputExtension(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "tasks.json") + + c, ui := newTestInitCommandUi() + if exit := c.Run([]string{"--output", path, "--format", "yaml"}); exit != 0 { + t.Fatalf("exit = %d, want 0: %s", exit, ui.ErrorWriter.String()) + } + body, err := os.ReadFile(path) + if err != nil { + t.Fatalf("tasks.json not written: %v", err) + } + if !strings.HasPrefix(string(body), "---\n") { + t.Errorf("--format yaml should have won over the .json extension:\n%s", body) + } + if problems := tasks.Validate(body, tasks.ValidateOptions{}); len(problems) > 0 { + t.Errorf("scaffold did not validate as YAML: %+v", problems) + } + if warn := ui.ErrorWriter.String(); !strings.Contains(warn, "--tasks-format yaml") { + t.Errorf("a lying extension should warn how to read it back:\n%s", warn) + } +} + +// TestInitFormatJSON5ChecksForceOnAdjustedPath is the sequencing test: +// the default-path swap has to happen before the exists / --force check, +// or init would stat tasks.yml while writing tasks.json - refusing over +// an unrelated file, then clobbering the relevant one. +func TestInitFormatJSON5ChecksForceOnAdjustedPath(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + + jsonPath := filepath.Join(dir, "tasks.json") + yamlPath := filepath.Join(dir, "tasks.yml") + if err := os.WriteFile(jsonPath, []byte("preserved\n"), 0o644); err != nil { + t.Fatalf("seed tasks.json: %v", err) + } + if err := os.WriteFile(yamlPath, []byte("yaml-preserved\n"), 0o644); err != nil { + t.Fatalf("seed tasks.yml: %v", err) + } + + c, ui := newTestInitCommandUi() + if exit := c.Run([]string{"--format", "json5"}); exit != 1 { + t.Fatalf("exit = %d, want 1", exit) + } + if errOut := ui.ErrorWriter.String(); !strings.Contains(errOut, "tasks.json already exists") { + t.Errorf("error should name the adjusted path:\n%s", errOut) + } + if body, _ := os.ReadFile(jsonPath); string(body) != "preserved\n" { + t.Errorf("tasks.json was overwritten without --force: %q", body) + } + + if exit := newTestInitCommand().Run([]string{"--format", "json5", "--force"}); exit != 0 { + t.Fatalf("--force exit = %d, want 0", exit) + } + body, err := os.ReadFile(jsonPath) + if err != nil { + t.Fatalf("read tasks.json: %v", err) + } + if !strings.Contains(string(body), "dokku_app") { + t.Errorf("--force should have rewritten tasks.json:\n%s", body) + } + if yaml, _ := os.ReadFile(yamlPath); string(yaml) != "yaml-preserved\n" { + t.Errorf("tasks.yml is not the target and must be left alone: %q", yaml) + } +} + +// TestInitFormatJSON5ToStdout is the case #410 was filed for: before +// --format, `--output -` could only ever emit YAML. +func TestInitFormatJSON5ToStdout(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + + captured, exit := captureStdout(t, func() int { + return newTestInitCommand().Run([]string{"--output", "-", "--format", "json5", "--name", "demo"}) + }) + if exit != 0 { + t.Fatalf("exit = %d, want 0", exit) + } + if strings.HasPrefix(captured, "---") { + t.Errorf("JSON5 on stdout should not carry the YAML document marker:\n%s", captured) + } + if !strings.HasPrefix(captured, "[") { + t.Errorf("stdout should open with a JSON5 array:\n%s", captured) + } + if !strings.Contains(captured, "dokku_app") { + t.Errorf("stdout missing dokku_app:\n%s", captured) + } + if strings.Contains(captured, "==> Created") { + t.Errorf("stdout contains the success block (should be suppressed):\n%s", captured) + } + if _, err := tasks.UnmarshalRecipe([]byte(captured), tasks.FormatNameJSON5); err != nil { + t.Errorf("streamed scaffold did not parse as JSON5: %v", err) + } + for _, name := range []string{"tasks.yml", "tasks.json"} { + if _, err := os.Stat(filepath.Join(dir, name)); err == nil { + t.Errorf("--output - should not create %s on disk", name) + } + } +} + +// TestInitRejectsUnknownFormat checks the value error names the flag the +// user actually typed, not the --tasks-format it shares a parser with. +func TestInitRejectsUnknownFormat(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + + c, ui := newTestInitCommandUi() + if exit := c.Run([]string{"--format", "toml"}); exit != 1 { + t.Fatalf("exit = %d, want 1", exit) + } + errOut := ui.ErrorWriter.String() + if !strings.Contains(errOut, "--format") { + t.Errorf("error should name --format:\n%s", errOut) + } + if !strings.Contains(errOut, "yaml, json5") { + t.Errorf("error should name the valid values:\n%s", errOut) + } + for _, name := range []string{"tasks.yml", "tasks.json"} { + if _, err := os.Stat(filepath.Join(dir, name)); err == nil { + t.Errorf("a rejected --format should not create %s", name) + } + } +} + // newTestInitCommand wires up a Meta backed by cli.MockUi so c.Ui.* calls // don't nil-panic during Run. Tests assert via the file system or stdout // capture; MockUi's buffers are ignored. func newTestInitCommand() *InitCommand { - c := &InitCommand{} - c.Meta = command.Meta{Ui: cli.NewMockUi()} + c, _ := newTestInitCommandUi() return c } +// newTestInitCommandUi is newTestInitCommand for tests that also need to +// read what the command said - the mismatch warning lands on the UI's +// error buffer, not on stdout. +func newTestInitCommandUi() (*InitCommand, *cli.MockUi) { + ui := cli.NewMockUi() + c := &InitCommand{} + c.Meta = command.Meta{Ui: ui} + return c, ui +} + func readAllString(r io.Reader) (string, error) { b, err := io.ReadAll(r) return string(b), err diff --git a/commands/plan.go b/commands/plan.go index a32fd36..0c0e6f8 100644 --- a/commands/plan.go +++ b/commands/plan.go @@ -117,7 +117,7 @@ func (c *PlanCommand) AutocompleteFlags() complete.Flags { c.Meta.AutocompleteFlags(command.FlagSetClient), complete.Flags{ "--tasks": taskFileAutocomplete(), - "--tasks-format": tasksFormatAutocomplete(), + "--tasks-format": recipeFormatAutocomplete(), "--json": complete.PredictNothing, "--detailed-exitcode": complete.PredictNothing, "--host": complete.PredictAnything, @@ -163,7 +163,7 @@ func (c *PlanCommand) Run(args []string) int { resolvedHost := resolveSshFlags(c.host, c.sudo, c.acceptNewHostKeys) - formatOverride, err := parseTasksFormatFlag(c.tasksFormatFlag) + formatOverride, err := parseRecipeFormatFlag("--tasks-format", c.tasksFormatFlag) if err != nil { c.Ui.Error(err.Error()) return 1 diff --git a/commands/task_file.go b/commands/task_file.go index 823470b..bb0a28f 100644 --- a/commands/task_file.go +++ b/commands/task_file.go @@ -37,11 +37,16 @@ const taskFileStdin = "-" // fall through to give JSON-native users a no-config setup. var defaultTaskFileCandidates = []string{"tasks.yml", "tasks.yaml", "tasks.json"} -// parseTasksFormatFlag normalises a --tasks-format value to one of the -// two canonical format identifiers. An empty value means "not set" and -// leaves detection to taskFileFormatFor. Anything else is rejected +// parseRecipeFormatFlag normalises a recipe-format flag value to one of +// the two canonical format identifiers. An empty value means "not set" +// and leaves the decision to the caller. Anything else is rejected // naming the accepted values, the way an invalid --color is. -func parseTasksFormatFlag(value string) (string, error) { +// +// flagName is the spelling to blame in that rejection: the input side +// (apply / plan / validate / fmt) passes "--tasks-format", the output +// side (init / export, #410) passes "--format". One normaliser keeps the +// two flags accepting exactly the same spellings. +func parseRecipeFormatFlag(flagName, value string) (string, error) { switch strings.ToLower(strings.TrimSpace(value)) { case "": return "", nil @@ -50,14 +55,14 @@ func parseTasksFormatFlag(value string) (string, error) { case "json", "json5": return taskFileFormatJSON5, nil } - return "", fmt.Errorf("invalid --tasks-format %q: must be one of yaml, json5", value) + return "", fmt.Errorf("invalid %s %q: must be one of yaml, json5", flagName, value) } // taskFileFormatFor resolves the format of a recipe from the three // signals available, in precedence order: // // 1. override - an explicit --tasks-format, already normalised by -// parseTasksFormatFlag +// parseRecipeFormatFlag // 2. detected - the extension of the path or URL, from // detectTaskFileFormat; empty when the source is stdin, which has no // name to key off @@ -109,6 +114,75 @@ func detectTaskFileFormat(path string) string { } } +// defaultRecipeOutput and defaultRecipeOutputJSON5 are the --output +// defaults for the two commands that write a recipe (init, export). +// resolveRecipeOutput swaps in the JSON5 spelling when --format json5 is +// given without an explicit --output, so the file name matches its +// contents and a later bare `docket validate` still finds it - both are +// in defaultTaskFileCandidates. +const ( + defaultRecipeOutput = "tasks.yml" + defaultRecipeOutputJSON5 = "tasks.json" +) + +// resolveRecipeOutput reconciles an explicit --format with the --output +// path a recipe-writing command is about to use, returning the path to +// write and the format to write it in. +// +// Precedence, per #410: +// +// 1. override - an explicit --format, already normalised by +// parseRecipeFormatFlag. It always wins, including over an --output +// extension that says otherwise. +// 2. the --output extension, via detectTaskFileFormat. +// 3. YAML - all that is left for stdout, which has no extension. This is +// the gap #410 exists to close: before --format, `--output -` could +// only ever emit YAML. +// +// outputChanged is flags.Changed("output"), which is only meaningful +// after flags.Parse. When --format asks for JSON5 and --output was left +// at its default, the default path moves to defaultRecipeOutputJSON5 +// rather than dropping a JSON5 document into a .yml file. A path the user +// typed - including "-" - is never rewritten. +// +// Callers must run this before their --force / --overwrite existence +// checks: those have to test the path that will actually be written. +func resolveRecipeOutput(output, override string, outputChanged bool) (string, string) { + if override == "" { + if output == taskFileStdin { + return output, taskFileFormatYAML + } + return output, detectTaskFileFormat(output) + } + if output != taskFileStdin && !outputChanged && override == taskFileFormatJSON5 { + output = defaultRecipeOutputJSON5 + } + return output, override +} + +// recipeOutputFormatMismatch returns the warning to print when --format +// disagrees with what path's extension implies, or "" when there is +// nothing to say. Writing a JSON5 recipe to tasks.yml is legal - --format +// always wins - but nothing downstream can tell: the JSON5 formatter +// emits unquoted keys, comments, and trailing commas, none of which parse +// as YAML, and a later `docket validate --tasks tasks.yml` picks its +// parser from the extension. Say it once, on stderr, instead of letting +// the user find out from a parse error. +// +// The companion vars-file deliberately gets no warning: MarshalVars emits +// plain JSON, which is valid YAML, so a .yml vars-file holding JSON still +// loads. +func recipeOutputFormatMismatch(path, override string) string { + if override == "" || path == taskFileStdin { + return "" + } + if detectTaskFileFormat(path) == override { + return "" + } + return fmt.Sprintf("warning: --format %s does not match the %s extension; reading %s back needs --tasks-format %s", + override, path, path, override) +} + // taskFileFetchTimeout bounds a remote recipe fetch so a hung server does // not stall the whole command. const taskFileFetchTimeout = 30 * time.Second @@ -367,8 +441,8 @@ func preloadRecipeForFlags(argv []string, allowURL bool) (data []byte, format st } // The override is read straight from argv: pflag has not run, so // the command's flag field is still empty. An unrecognised value is - // ignored here and rejected properly by parseTasksFormatFlag in Run. - override, _ := parseTasksFormatFlag(tasksFormatFromArgs(argv)) + // ignored here and rejected properly by parseRecipeFormatFlag in Run. + override, _ := parseRecipeFormatFlag("--tasks-format", tasksFormatFromArgs(argv)) return data, taskFileFormatFor(detected, override, data), path } @@ -403,10 +477,11 @@ func predictFilesByExtension(extensions []string) complete.Predictor { }) } -// tasksFormatAutocomplete offers the two canonical --tasks-format -// values. The yml / json aliases parseTasksFormatFlag also accepts are +// recipeFormatAutocomplete offers the two canonical values shared by +// --tasks-format on the reading side and --format on the writing side. +// The yml / json aliases parseRecipeFormatFlag also accepts are // deliberately left out so completion suggests one spelling per format. -func tasksFormatAutocomplete() complete.Predictor { +func recipeFormatAutocomplete() complete.Predictor { return complete.PredictSet(taskFileFormatYAML, taskFileFormatJSON5) } diff --git a/commands/task_file_test.go b/commands/task_file_test.go index 8772cc8..78cbfce 100644 --- a/commands/task_file_test.go +++ b/commands/task_file_test.go @@ -31,7 +31,7 @@ func TestDetectTaskFileFormat(t *testing.T) { } } -func TestParseTasksFormatFlag(t *testing.T) { +func TestParseRecipeFormatFlag(t *testing.T) { valid := map[string]string{ "": "", "yaml": taskFileFormatYAML, @@ -43,21 +43,33 @@ func TestParseTasksFormatFlag(t *testing.T) { " yaml": taskFileFormatYAML, } for value, want := range valid { - got, err := parseTasksFormatFlag(value) + got, err := parseRecipeFormatFlag("--tasks-format", value) if err != nil { - t.Errorf("parseTasksFormatFlag(%q) returned error: %v", value, err) + t.Errorf("parseRecipeFormatFlag(%q) returned error: %v", value, err) continue } if got != want { - t.Errorf("parseTasksFormatFlag(%q) = %q, want %q", value, got, want) + t.Errorf("parseRecipeFormatFlag(%q) = %q, want %q", value, got, want) } } for _, value := range []string{"toml", "hcl", "ini", "yamlish"} { - if _, err := parseTasksFormatFlag(value); err == nil { - t.Errorf("parseTasksFormatFlag(%q) = nil error, want a rejection", value) + if _, err := parseRecipeFormatFlag("--tasks-format", value); err == nil { + t.Errorf("parseRecipeFormatFlag(%q) = nil error, want a rejection", value) } else if !strings.Contains(err.Error(), "yaml, json5") { - t.Errorf("parseTasksFormatFlag(%q) error %q should name the valid values", value, err) + t.Errorf("parseRecipeFormatFlag(%q) error %q should name the valid values", value, err) + } + } + + // The flag name in the message follows the caller, so --format and + // --tasks-format each blame themselves rather than the other. + for _, flagName := range []string{"--tasks-format", "--format"} { + _, err := parseRecipeFormatFlag(flagName, "toml") + if err == nil { + t.Fatalf("parseRecipeFormatFlag(%q, toml) = nil error, want a rejection", flagName) + } + if !strings.Contains(err.Error(), flagName) { + t.Errorf("error %q should name the flag it rejects (%s)", err, flagName) } } } @@ -93,6 +105,94 @@ func TestTaskFileFormatFor(t *testing.T) { } } +// TestResolveRecipeOutput pins the output-side precedence rule (#410): +// an explicit --format beats the --output extension, which beats the +// YAML fallback stdout is left with. It also pins the one case where the +// format changes the path: --format json5 with an untouched --output +// moves the default to tasks.json rather than writing a JSON5 document +// into a file named tasks.yml. +func TestResolveRecipeOutput(t *testing.T) { + tests := []struct { + name string + output string + override string + outputChanged bool + wantPath string + wantFormat string + }{ + {name: "default with no override", output: defaultRecipeOutput, wantPath: defaultRecipeOutput, wantFormat: taskFileFormatYAML}, + {name: "default with json5 moves the path", output: defaultRecipeOutput, override: taskFileFormatJSON5, wantPath: defaultRecipeOutputJSON5, wantFormat: taskFileFormatJSON5}, + {name: "default with yaml stays put", output: defaultRecipeOutput, override: taskFileFormatYAML, wantPath: defaultRecipeOutput, wantFormat: taskFileFormatYAML}, + {name: "explicit path is never rewritten", output: "deploy/prod.yml", override: taskFileFormatJSON5, outputChanged: true, wantPath: "deploy/prod.yml", wantFormat: taskFileFormatJSON5}, + {name: "explicit default path is never rewritten", output: defaultRecipeOutput, override: taskFileFormatJSON5, outputChanged: true, wantPath: defaultRecipeOutput, wantFormat: taskFileFormatJSON5}, + {name: "override beats a json extension", output: "tasks.json", override: taskFileFormatYAML, outputChanged: true, wantPath: "tasks.json", wantFormat: taskFileFormatYAML}, + {name: "extension decides with no override", output: "tasks.json5", outputChanged: true, wantPath: "tasks.json5", wantFormat: taskFileFormatJSON5}, + {name: "unknown extension falls back to yaml", output: "recipe.txt", outputChanged: true, wantPath: "recipe.txt", wantFormat: taskFileFormatYAML}, + {name: "stdin with no override is yaml", output: taskFileStdin, outputChanged: true, wantPath: taskFileStdin, wantFormat: taskFileFormatYAML}, + {name: "stdin honours the override", output: taskFileStdin, override: taskFileFormatJSON5, outputChanged: true, wantPath: taskFileStdin, wantFormat: taskFileFormatJSON5}, + {name: "stdin is never rewritten to a path", output: taskFileStdin, override: taskFileFormatJSON5, wantPath: taskFileStdin, wantFormat: taskFileFormatJSON5}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotPath, gotFormat := resolveRecipeOutput(tt.output, tt.override, tt.outputChanged) + if gotPath != tt.wantPath || gotFormat != tt.wantFormat { + t.Errorf("resolveRecipeOutput(%q, %q, %t) = (%q, %q), want (%q, %q)", + tt.output, tt.override, tt.outputChanged, gotPath, gotFormat, tt.wantPath, tt.wantFormat) + } + }) + } +} + +// TestResolveRecipeOutputDefaultsAreProbeCandidates is what makes the +// path swap safe: init tells the user to run a bare `docket validate` +// next, and that probes defaultTaskFileCandidates. A default output that +// is not in that list would leave the scaffold unreachable. +func TestResolveRecipeOutputDefaultsAreProbeCandidates(t *testing.T) { + for _, want := range []string{defaultRecipeOutput, defaultRecipeOutputJSON5} { + found := false + for _, candidate := range defaultTaskFileCandidates { + if candidate == want { + found = true + break + } + } + if !found { + t.Errorf("%q is a default --output but not in defaultTaskFileCandidates %v", want, defaultTaskFileCandidates) + } + } +} + +// TestRecipeOutputFormatMismatch pins when the extension-lies warning +// fires. Only a recipe whose extension disagrees with --format earns +// one; stdout has no extension to disagree with, and no --format means +// the extension was the source of truth in the first place. +func TestRecipeOutputFormatMismatch(t *testing.T) { + quiet := [][2]string{ + {"tasks.yml", ""}, + {taskFileStdin, taskFileFormatJSON5}, + {"tasks.json", taskFileFormatJSON5}, + {"tasks.json5", taskFileFormatJSON5}, + {"tasks.yml", taskFileFormatYAML}, + {"recipe.txt", taskFileFormatYAML}, + } + for _, c := range quiet { + if got := recipeOutputFormatMismatch(c[0], c[1]); got != "" { + t.Errorf("recipeOutputFormatMismatch(%q, %q) = %q, want no warning", c[0], c[1], got) + } + } + + got := recipeOutputFormatMismatch("tasks.yml", taskFileFormatJSON5) + if got == "" { + t.Fatal("recipeOutputFormatMismatch(tasks.yml, json5) = no warning, want one") + } + if !strings.Contains(got, "tasks.yml") { + t.Errorf("warning %q should name the path", got) + } + if !strings.Contains(got, "--tasks-format json5") { + t.Errorf("warning %q should name the flag needed to read it back", got) + } +} + func TestTaskFileDisplayName(t *testing.T) { cases := map[string]string{ taskFileStdin: "", diff --git a/commands/validate.go b/commands/validate.go index 2840021..b32256b 100644 --- a/commands/validate.go +++ b/commands/validate.go @@ -105,7 +105,7 @@ func (c *ValidateCommand) AutocompleteFlags() complete.Flags { c.Meta.AutocompleteFlags(command.FlagSetClient), complete.Flags{ "--tasks": taskFileAutocomplete(), - "--tasks-format": tasksFormatAutocomplete(), + "--tasks-format": recipeFormatAutocomplete(), "--json": complete.PredictNothing, "--strict": complete.PredictNothing, "--vars-file": complete.PredictFiles("*"), @@ -142,7 +142,7 @@ func (c *ValidateCommand) Run(args []string) int { return 1 } - formatOverride, err := parseTasksFormatFlag(c.tasksFormatFlag) + formatOverride, err := parseRecipeFormatFlag("--tasks-format", c.tasksFormatFlag) if err != nil { if c.json { c.emitJSONProblem(tasks.Problem{ diff --git a/docs/command-reference.md b/docs/command-reference.md index c49c1ed..dda806e 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -28,6 +28,7 @@ recipe declares still become real `--` flags, and a `-` recipe takes prece docket export --output - | docket apply - docket init --output - | docket validate - cat tasks.yml | docket plan --tasks - --app api +docket export --output - --format json5 | docket apply --tasks-format json5 - ``` The format normally comes from the file extension, and for stdin from the first non-whitespace byte @@ -36,6 +37,10 @@ overrides both. Reach for it when the extension is absent or misleading (`--task URL whose path carries no extension), or when a YAML recipe written in flow style would be sniffed as JSON5 because it opens with `[`. +`--tasks-format` is the reading side. On the writing side, `init` and `export` take +`--format yaml|json5` to state the format of what they emit. Without it, stdout can only ever be +YAML, since there is no extension to infer from. + Reading the recipe from stdin consumes it, so a `dokku` command that would otherwise have inherited the terminal's stdin sees end-of-file instead. No task depends on this - every task that streams data to `dokku` supplies it explicitly. @@ -47,24 +52,31 @@ contact and no `git` subprocess. The default scaffold ships four tasks (`dokku_a `dokku_config`, `dokku_domains`, `dokku_git_sync`) in a single play with `app` and `repo` inputs, and round-trips cleanly through `docket validate`. -The output format follows the `--output` extension: `.json` / `.json5` writes a JSON5 scaffold with -`// ...` comments, anything else writes YAML. Streaming to stdout (`--output -`) writes YAML. +The output format follows `--format` when given, otherwise the `--output` extension: `.json` / +`.json5` writes a JSON5 scaffold with `// ...` comments, anything else writes YAML. Streaming to +stdout (`--output -`) has no extension to read, so it writes YAML unless `--format json5` says +otherwise. Passing `--format json5` without an `--output` writes `./tasks.json` rather than a JSON5 +document under a `.yml` name. ```bash # Use the current directory name as the app and remote.origin.url as the repo. docket init -# Same scaffold in JSON5. -docket init --output tasks.json +# Same scaffold in JSON5, written to ./tasks.json. +docket init --format json5 # Stream the scaffold to stdout for piping. docket init --output - + +# Stream a JSON5 scaffold to stdout. +docket init --output - --format json5 ``` | Flag | Effect | |------|--------| | (default) | Write `./tasks.yml`; refuse if it already exists. | -| `--output ` | Write to a path; `-` writes to stdout. Format inferred from the extension. | +| `--output ` | Write to a path; `-` writes to stdout. Format inferred from the extension unless `--format` says otherwise. | +| `--format ` | Write `yaml` or `json5` regardless of the `--output` extension. Without an explicit `--output`, `--format json5` writes `./tasks.json`. | | `--force` | Overwrite an existing file. | | `--name ` | Set the play and `app` input default (defaults to the directory name). | | `--repo ` | Set the `repo` input default (defaults to `remote.origin.url` in `./.git/config`). | @@ -335,6 +347,9 @@ docket export --host deploy@dokku.example.com # Apply the exported pair somewhere else. docket apply --tasks tasks.yml --vars-file tasks.vars.yml + +# Stream a JSON5 recipe to stdout and pipe it straight back in. +docket export --output - --format json5 | docket apply --tasks-format json5 - ``` The correctness contract is idempotency: applying an exported pair back to the same server reports @@ -343,6 +358,7 @@ no drift (`plan` shows every task `[ok]`). | Flag | Effect | |------|--------| | `--output ` | Where to write the recipe (default `tasks.yml`). Pass `-` to stream a single self-contained recipe (values inlined, no vars-file) to stdout for inspection. | +| `--format ` | Write `yaml` or `json5` regardless of the `--output` extension, for both the recipe and the vars-file. Without an explicit `--output`, `--format json5` writes `./tasks.json` and `./tasks.vars.json`. Required to stream JSON5 with `--output -`, which has no extension to read. | | `--vars-output ` | Where to write the companion vars-file (default `.vars.`, e.g. `tasks.vars.yml`). | | `--overwrite` | Overwrite existing output files without prompting. Without it, export prompts before replacing either file, and aborts writing nothing if declined (or if stdin is not interactive). | | `--redact` | Write placeholder values into the vars-file instead of real secrets, producing a shareable recipe plus a fill-in-the-blanks vars template. The `required` inputs mean `apply` fails loudly until the vars-file is filled in. | @@ -351,8 +367,9 @@ no drift (`plan` shows every task `[ok]`). | `--sudo` | Wrap the remote `dokku` call in `sudo -n`. | | `--accept-new-host-keys` | Trust an unknown SSH host key on first connect. | -The output format follows the `--output` extension (`.json` / `.json5` writes JSON5, anything else -YAML), and the vars-file matches. Which task types export is a per-task property: each task's +The output format follows `--format` when given, otherwise the `--output` extension (`.json` / +`.json5` writes JSON5, anything else YAML); the vars-file follows the recipe, or its own +`--vars-output` extension when `--format` is not given. Which task types export is a per-task property: each task's reference page carries an **Export support** section stating whether it is supported, partial (for example a value that is lifted into the vars-file), or not exportable (write-only credentials such as `dokku_git_auth`, or `dokku_service_property`, which no datastore plugin can read back). diff --git a/docs/recipes.md b/docs/recipes.md index bbe8c4a..bf1823f 100644 --- a/docs/recipes.md +++ b/docs/recipes.md @@ -67,6 +67,7 @@ it a recipe another tool just generated: ```bash docket export --output - | docket apply - docket init --output - | docket validate - +docket export --output - --format json5 | docket apply --tasks-format json5 - ``` The format is sniffed from the first non-whitespace byte - `[`, `{`, `//`, or `/*` means JSON5, @@ -78,6 +79,10 @@ flag overrides a misleading file extension: docket validate --tasks recipe.txt --tasks-format json5 ``` +`--tasks-format` is the reading side. The writing side is `--format` on `init` and `export`, which +states the format of what they emit - necessary when the destination is `-`, since there is no +extension to infer from. + A piped recipe behaves like any other: its `inputs:` still become `--` flags, and it wins over a `tasks.yml` in the current directory. diff --git a/tests/bats/completion.bats b/tests/bats/completion.bats index 0cacb31..ce1071c 100644 --- a/tests/bats/completion.bats +++ b/tests/bats/completion.bats @@ -43,6 +43,25 @@ setup() { assert_output --partial 'json5' } +@test "docket init --format completes the two recipe formats" { + cd "$BATS_TEST_TMPDIR" + export COMP_LINE='docket init --format ' + run "$(docket_bin)" init --format + assert_success + assert_output --partial 'yaml' + assert_output --partial 'json5' +} + +@test "docket export --format completes the two recipe formats" { + # Completion short-circuits before Run, so this needs no server. + cd "$BATS_TEST_TMPDIR" + export COMP_LINE='docket export --format ' + run "$(docket_bin)" export --format + assert_success + assert_output --partial 'yaml' + assert_output --partial 'json5' +} + @test "docket fmt completes recipe files positionally (#340)" { cd "$BATS_TEST_TMPDIR" : >tasks.yml diff --git a/tests/bats/export.bats b/tests/bats/export.bats index 41cb3bf..455f922 100644 --- a/tests/bats/export.bats +++ b/tests/bats/export.bats @@ -36,3 +36,32 @@ teardown() { assert_success assert_output --partial "(1 app)" } + +@test "docket export --format json5 writes a tasks.json / tasks.vars.json pair" { + dokku apps:create docket-test-export + # A config value is what gets lifted into the vars-file; without one + # the export has no vars and writes no companion file at all. + dokku config:set --no-restart docket-test-export API_KEY=abc123 + cd "$BATS_TEST_TMPDIR" + run "$(docket_bin)" export --app docket-test-export --format json5 + assert_success + assert [ -f tasks.json ] + assert [ -f tasks.vars.json ] + assert [ ! -f tasks.yml ] + + run head -1 tasks.json + assert_output "[" + + run "$(docket_bin)" validate --tasks tasks.json --vars-file tasks.vars.json + assert_success + assert_output --partial "is valid" +} + +@test "docket export --output - --format json5 round-trips into apply" { + dokku apps:create docket-test-export + # The motivating command from #410, with --list-tasks so the pipe is + # exercised without applying anything back to the server. + run bash -c "\"$(docket_bin)\" export --app docket-test-export --output - --format json5 | \"$(docket_bin)\" apply --tasks-format json5 --list-tasks -" + assert_success + assert_output --partial "docket-test-export" +} diff --git a/tests/bats/init.bats b/tests/bats/init.bats index 577ace9..36e27e2 100644 --- a/tests/bats/init.bats +++ b/tests/bats/init.bats @@ -125,6 +125,68 @@ CFG refute_output --partial "inputs:" } +@test "docket init --format json5 writes tasks.json" { + cd "$BATS_TEST_TMPDIR" + run "$(docket_bin)" init --format json5 --name api --repo https://example.com/repo.git + assert_success + assert_output --partial "Created tasks.json" + assert [ -f tasks.json ] + assert [ ! -f tasks.yml ] + + run head -1 tasks.json + assert_output "[" + + run "$(docket_bin)" validate --tasks tasks.json + assert_success + assert_output --partial "is valid" +} + +@test "docket init --format json5 output validates with no --tasks" { + # tasks.json is in the probe list, so the Next steps block's bare + # `docket validate` still finds a JSON5 scaffold. + cd "$BATS_TEST_TMPDIR" + "$(docket_bin)" init --format json5 --name api --repo https://example.com/repo.git + run "$(docket_bin)" validate + assert_success + assert_output --partial "is valid" +} + +@test "docket init --format yaml keeps the tasks.yml default" { + cd "$BATS_TEST_TMPDIR" + run "$(docket_bin)" init --format yaml + assert_success + assert [ -f tasks.yml ] + assert [ ! -f tasks.json ] +} + +@test "docket init --output - --format json5 streams JSON5 to stdout" { + cd "$BATS_TEST_TMPDIR" + run bash -c "\"$(docket_bin)\" init --output - --format json5 --name api --repo https://example.com/repo.git | head -1" + assert_success + assert_output "[" + assert [ ! -f tasks.yml ] + assert [ ! -f tasks.json ] +} + +@test "docket init --format json5 checks --force against tasks.json" { + cd "$BATS_TEST_TMPDIR" + echo "preserved" >tasks.json + run "$(docket_bin)" init --format json5 + assert_failure + assert_output --partial "tasks.json already exists" + run cat tasks.json + assert_output "preserved" +} + +@test "docket init rejects an unknown --format" { + cd "$BATS_TEST_TMPDIR" + run "$(docket_bin)" init --format toml + assert_failure + assert_output --partial "yaml, json5" + assert [ ! -f tasks.yml ] + assert [ ! -f tasks.json ] +} + @test "docket init --name sets the play name so --play resolves" { cd "$BATS_TEST_TMPDIR" run "$(docket_bin)" init --name web --minimal diff --git a/tests/bats/stdin.bats b/tests/bats/stdin.bats index 86b77cd..72b7f24 100644 --- a/tests/bats/stdin.bats +++ b/tests/bats/stdin.bats @@ -90,6 +90,15 @@ EOF assert_output --partial "is valid" } +@test "docket init --output - --format json5 pipes into docket validate --tasks-format json5 -" { + # The writing side of the round trip #410 added: --format states the + # format of what init emits, --tasks-format states how validate reads it. + cd "$BATS_TEST_TMPDIR" + run bash -c "\"$(docket_bin)\" init --output - --format json5 --name api --repo https://example.com/repo.git | \"$(docket_bin)\" validate --tasks-format json5 -" + assert_success + assert_output --partial "is valid" +} + @test "docket apply --list-tasks reads a recipe from stdin" { cd "$BATS_TEST_TMPDIR" cat >input.yml <<'EOF'