diff --git a/checks/cli.go b/checks/cli.go index 4dc95df..f9a2e8d 100644 --- a/checks/cli.go +++ b/checks/cli.go @@ -15,6 +15,38 @@ import ( const maxCLIOutputBytesPerStream = 1024 * 1024 +type commandShell struct { + path string + commandFlag string +} + +func defaultShell() commandShell { + if runtime.GOOS == "windows" { + return commandShell{path: "powershell", commandFlag: "-Command"} + } + return commandShell{path: "sh", commandFlag: "-c"} +} + +func resolveShell(name string) (commandShell, error) { + if name == "" { + return defaultShell(), nil + } + var flag string + switch name { + case "sh": + flag = "-c" + case "pwsh": + flag = "-Command" + default: + return commandShell{}, fmt.Errorf("unsupported shell %q: choose sh or pwsh", name) + } + path, err := exec.LookPath(name) + if err != nil { + return commandShell{}, fmt.Errorf("shell %q is unavailable: %w", name, err) + } + return commandShell{path: path, commandFlag: flag}, nil +} + type boundedBuffer struct { buffer bytes.Buffer limit int @@ -44,25 +76,21 @@ func (b *boundedBuffer) String() string { return b.buffer.String() } -func runCLICommand(command api.CLIStepCLICommand, variables map[string]string) (result api.CLICommandResult) { - return runCLICommandWithOutputLimit(command, variables, maxCLIOutputBytesPerStream) +func runCLICommand(command api.CLIStepCLICommand, variables map[string]string, shell commandShell) (result api.CLICommandResult) { + return runCLICommandWithOutputLimit(command, variables, maxCLIOutputBytesPerStream, shell) } func runCLICommandWithOutputLimit( command api.CLIStepCLICommand, variables map[string]string, maxOutputBytesPerStream int, + shell commandShell, ) (result api.CLICommandResult) { finalCommand := InterpolateVariables(command.Command, variables) result.FinalCommand = finalCommand result.Command = command - var cmd *exec.Cmd - if runtime.GOOS == "windows" { - cmd = exec.Command("powershell", "-Command", finalCommand) - } else { - cmd = exec.Command("sh", "-c", finalCommand) - } + cmd := exec.Command(shell.path, shell.commandFlag, finalCommand) cmd.Env = append(os.Environ(), "LANG=en_US.UTF-8") stdout := newBoundedBuffer(maxOutputBytesPerStream) diff --git a/checks/cli_test.go b/checks/cli_test.go index 8752776..8073b29 100644 --- a/checks/cli_test.go +++ b/checks/cli_test.go @@ -25,6 +25,7 @@ func TestRunCLICommandCapsOutput(t *testing.T) { }, variables, 4, + defaultShell(), ) if !strings.Contains(result.Err, "per-stream limit") { @@ -49,7 +50,7 @@ func TestRunCLICommandCapturesStdoutVariables(t *testing.T) { Name: "goos", Regex: `([a-z0-9]+)`, }}, - }, variables) + }, variables, defaultShell()) if result.Err != "" { t.Fatalf("unexpected command error: %s", result.Err) @@ -80,7 +81,7 @@ func TestRunCLICommandKeepsStderrSeparateFromStdoutChecks(t *testing.T) { }}, } - result := runCLICommand(step, variables) + result := runCLICommand(step, variables, defaultShell()) if result.Stdout != "stdout-value" { t.Fatalf("stdout = %q, want stdout-value", result.Stdout) @@ -108,20 +109,20 @@ func TestRunCLICommandInterpolatesCapturedStdoutVariables(t *testing.T) { Name: "goenv", Regex: `"([A-Z]+)"`, }}, - }, variables) + }, variables, defaultShell()) if first.Err != "" { t.Fatalf("unexpected first command error: %s", first.Err) } second := runCLICommand(api.CLIStepCLICommand{ Command: `go env ${goenv}`, - }, variables) + }, variables, defaultShell()) if second.Stdout != runtime.GOOS { t.Fatalf("second stdout = %q, want %q", second.Stdout, runtime.GOOS) } } -func TestParseStdoutVariablesUsesGenericConfigurationError(t *testing.T) { +func TestParseStdoutVariablesRejectsInvalidConfiguration(t *testing.T) { tests := []struct { name string vardef api.CLICommandStdoutVariable @@ -151,9 +152,6 @@ func TestParseStdoutVariablesUsesGenericConfigurationError(t *testing.T) { if err == nil { t.Fatal("expected parse error") } - if err.Error() != "invalid stdout variable configuration" { - t.Fatalf("error = %q, want invalid stdout variable configuration", err.Error()) - } }) } } diff --git a/checks/local.go b/checks/local.go index e814743..650c344 100644 --- a/checks/local.go +++ b/checks/local.go @@ -3,11 +3,13 @@ package checks import ( "fmt" "math" + "math/big" "reflect" "strconv" "strings" api "github.com/bootdotdev/bootdev/client" + "github.com/goccy/go-json" ) func LocalSubmissionEvent(cliData api.CLIData, results []api.CLIStepResult) api.LessonSubmissionEvent { @@ -262,50 +264,96 @@ func evaluateStdoutJq(stdout string, test api.StdoutJqTest, variables map[string if err != nil { return err } - if len(results) != len(test.ExpectedResults) { - return fmt.Errorf("expected jq query %q to return %d result(s), got %d", queryText, len(test.ExpectedResults), len(results)) + if len(results) == 0 { + return fmt.Errorf("jq query returned no results") } - for i, expected := range test.ExpectedResults { - want, err := jqExpectedValue(expected, variables) - if err != nil { - return err +outer: + for _, expected := range test.ExpectedResults { + if value, ok := expected.Value.(string); ok { + expected.Value = InterpolateVariables(value, variables) } - if !compareValues(results[i], api.OperatorType(expected.Operator), want) { - return fmt.Errorf("expected jq result %d to be %s %v, got %v", i+1, expected.Operator, want, results[i]) + for _, actual := range results { + if jqResultMatches(actual, expected) { + continue outer + } } + return fmt.Errorf("expected jq results to contain %v", expected) } return nil } -func jqExpectedValue(expected api.JqExpectedResult, variables map[string]string) (any, error) { +func jqResultMatches(actual any, expected api.JqExpectedResult) bool { switch expected.Type { case api.JqTypeString: - if str, ok := expected.Value.(string); ok { - return InterpolateVariables(str, variables), nil - } - return expected.Value, nil + got, gotOK := actual.(string) + want, wantOK := expected.Value.(string) + return gotOK && wantOK && expected.Operator == "==" && got == want + case api.JqTypeBool: + got, gotOK := coerceJqBool(actual) + want, wantOK := coerceJqBool(expected.Value) + return gotOK && wantOK && expected.Operator == "==" && got == want case api.JqTypeInt: - if str, ok := expected.Value.(string); ok { - parsed, err := strconv.Atoi(InterpolateVariables(str, variables)) - if err != nil { - return nil, err - } - return parsed, nil + got, gotOK := coerceJqInt(actual) + want, wantOK := coerceJqInt(expected.Value) + if !gotOK || !wantOK { + return false } - return expected.Value, nil - case api.JqTypeBool: - if str, ok := expected.Value.(string); ok { - parsed, err := strconv.ParseBool(InterpolateVariables(str, variables)) - if err != nil { - return nil, err - } - return parsed, nil + switch expected.Operator { + case "==": + return got == want + case ">": + return got > want + case ">=": + return got >= want + case "<": + return got < want + case "<=": + return got <= want } - return expected.Value, nil + } + return false +} + +func coerceJqBool(value any) (bool, bool) { + switch v := value.(type) { + case bool: + return v, true + case string: + parsed, err := strconv.ParseBool(v) + return parsed, err == nil default: - return nil, fmt.Errorf("unsupported jq expected result type %q", expected.Type) + return false, false + } +} + +func coerceJqInt(value any) (int, bool) { + switch v := value.(type) { + case int: + return v, true + case int64: + if v < math.MinInt || v > math.MaxInt { + return 0, false + } + return int(v), true + case float64: + // MaxInt rounds up as float64 on 64-bit hosts; use an exclusive upper bound. + if math.IsNaN(v) || math.Trunc(v) != v || v < float64(math.MinInt) || v >= -float64(math.MinInt) { + return 0, false + } + return int(v), true + case json.Number: + parsed, ok := new(big.Rat).SetString(v.String()) + if !ok || !parsed.IsInt() || !parsed.Num().IsInt64() { + return 0, false + } + return coerceJqInt(parsed.Num().Int64()) + case string: + parsed, err := strconv.Atoi(v) + return parsed, err == nil + default: + return 0, false } } @@ -313,10 +361,22 @@ func compareValues(got any, operator api.OperatorType, want any) bool { switch operator { case api.OpEquals, "==": return valuesEqual(got, want) - case api.OpGreaterThan, ">": + case api.OpGreaterThan, ">", ">=", "<", "<=": gotNum, gotOK := numberValue(got) wantNum, wantOK := numberValue(want) - return gotOK && wantOK && gotNum > wantNum + if !gotOK || !wantOK { + return false + } + switch operator { + case api.OpGreaterThan, ">": + return gotNum > wantNum + case ">=": + return gotNum >= wantNum + case "<": + return gotNum < wantNum + case "<=": + return gotNum <= wantNum + } case api.OpContains: return strings.Contains(fmt.Sprintf("%v", got), fmt.Sprintf("%v", want)) case api.OpNotContains: @@ -324,6 +384,7 @@ func compareValues(got any, operator api.OperatorType, want any) bool { default: return false } + return false } func valuesEqual(got any, want any) bool { diff --git a/checks/local_test.go b/checks/local_test.go index 9dd50cc..a869d12 100644 --- a/checks/local_test.go +++ b/checks/local_test.go @@ -1,6 +1,8 @@ package checks import ( + "math" + "strconv" "testing" api "github.com/bootdotdev/bootdev/client" @@ -91,20 +93,33 @@ func TestEvaluateCLICommandReportsExecutionError(t *testing.T) { } } -func TestEvaluateStdoutJq(t *testing.T) { - err := evaluateStdoutJq( - "{\"ok\":true}", - api.StdoutJqTest{ - InputMode: "json", - Query: ".ok", - ExpectedResults: []api.JqExpectedResult{ - {Type: api.JqTypeBool, Operator: "==", Value: true}, - }, - }, - map[string]string{}, - ) - if err != nil { - t.Fatalf("unexpected jq failure: %v", err) +func TestEvaluateStdoutJqNumericComparisons(t *testing.T) { + for _, tt := range []struct { + operator api.JqOperator + pass [3]bool + }{ + {"==", [3]bool{false, true, false}}, + {">", [3]bool{false, false, true}}, + {">=", [3]bool{false, true, true}}, + {"<", [3]bool{true, false, false}}, + {"<=", [3]bool{true, true, false}}, + } { + for i, stdout := range []string{"4", "5", "6"} { + t.Run(stdout+string(tt.operator)+"5", func(t *testing.T) { + err := evaluateStdoutJq(stdout, api.StdoutJqTest{ + InputMode: "json", + Query: ".", + ExpectedResults: []api.JqExpectedResult{{ + Type: api.JqTypeInt, + Operator: tt.operator, + Value: 5, + }}, + }, nil) + if (err == nil) != tt.pass[i] { + t.Fatalf("comparison passed = %t, want %t; error: %v", err == nil, tt.pass[i], err) + } + }) + } } } @@ -277,3 +292,77 @@ func intPtr(v int) *int { func stringPtr(v string) *string { return &v } + +func TestEvaluateStdoutJqMatchesAnyResult(t *testing.T) { + for _, tt := range []struct { + name string + stdout string + expected []int + pass bool + }{ + {"extra and reordered results", "[3, 2, 1]", []int{1, 2}, true}, + {"reuse an actual result", "[1]", []int{1, 1}, true}, + {"missing expected result", "[1, 3]", []int{1, 2}, false}, + {"empty results", "[]", []int{1}, false}, + {"empty results without expectations", "[]", nil, false}, + {"nonempty results without expectations", "[1]", nil, true}, + } { + t.Run(tt.name, func(t *testing.T) { + test := api.StdoutJqTest{InputMode: "json", Query: ".[]"} + for _, value := range tt.expected { + test.ExpectedResults = append(test.ExpectedResults, api.JqExpectedResult{ + Type: api.JqTypeInt, Operator: "==", Value: value, + }) + } + err := evaluateStdoutJq(tt.stdout, test, nil) + if (err == nil) != tt.pass { + t.Fatalf("passed = %t, want %t; error: %v", err == nil, tt.pass, err) + } + }) + } +} + +func TestEvaluateStdoutJqResultTypes(t *testing.T) { + for _, tt := range []struct { + name string + stdout string + kind api.JqValueType + operator api.JqOperator + want any + pass bool + }{ + {"numeric string", `"5"`, api.JqTypeInt, "==", 5, true}, + {"interpolated integer", "5", api.JqTypeInt, "==", "${value}", true}, + {"integral expected float", "5", api.JqTypeInt, "==", 5.0, true}, + {"decimal JSON number", "5.0", api.JqTypeInt, "==", 5, true}, + {"exponent JSON number", "5e0", api.JqTypeInt, "==", 5, true}, + {"tiny fractional part", "5.0000000000000000001", api.JqTypeInt, "==", 5, false}, + {"exact maximum integer", strconv.Itoa(math.MaxInt) + ".0", api.JqTypeInt, "==", math.MaxInt, true}, + {"JSON integer overflow", "9223372036854775808.0", api.JqTypeInt, ">", 0, false}, + {"fractional actual", "5.5", api.JqTypeInt, ">", 5, false}, + {"fractional expected", "6", api.JqTypeInt, ">", 5.5, false}, + {"exact large integer", "9007199254740992", api.JqTypeInt, "==", json.Number("9007199254740993"), false}, + {"out of range float", "0", api.JqTypeInt, "<=", -float64(math.MinInt), false}, + {"boolean", "true", api.JqTypeBool, "==", true, true}, + {"boolean strings", `"true"`, api.JqTypeBool, "==", "true", true}, + {"invalid boolean", `"yes"`, api.JqTypeBool, "==", true, false}, + {"interpolated string", `"5"`, api.JqTypeString, "==", "${value}", true}, + {"string type rejects numbers", "5", api.JqTypeString, "==", 5, false}, + {"boolean type rejects numbers", "1", api.JqTypeBool, "==", 1, false}, + {"string ordering unsupported", `"b"`, api.JqTypeString, ">", "a", false}, + {"unknown operator", "5", api.JqTypeInt, "!=", 4, false}, + } { + t.Run(tt.name, func(t *testing.T) { + err := evaluateStdoutJq(tt.stdout, api.StdoutJqTest{ + InputMode: "json", + Query: ".", + ExpectedResults: []api.JqExpectedResult{{ + Type: tt.kind, Operator: tt.operator, Value: tt.want, + }}, + }, map[string]string{"value": "5"}) + if (err == nil) != tt.pass { + t.Fatalf("passed = %t, want %t; error: %v", err == nil, tt.pass, err) + } + }) + } +} diff --git a/checks/runner.go b/checks/runner.go index 8e2424b..9f0632c 100644 --- a/checks/runner.go +++ b/checks/runner.go @@ -13,7 +13,17 @@ import ( const lessonHTTPRequestTimeout = 30 * time.Second -func CLIChecks(cliData api.CLIData, overrideBaseURL string, send func(tea.Msg)) ([]api.CLIStepResult, error) { +type RunOptions struct { + OverrideBaseURL string + Shell string +} + +func CLIChecks(cliData api.CLIData, options RunOptions, send func(tea.Msg)) ([]api.CLIStepResult, error) { + shell, err := resolveShell(options.Shell) + if err != nil { + return nil, err + } + overrideBaseURL := options.OverrideBaseURL if cliData.BaseURLDefault == api.BaseURLOverrideRequired && overrideBaseURL == "" { return nil, errors.New("lesson requires a base URL override: `bootdev configure base_url `") } @@ -41,7 +51,7 @@ func CLIChecks(cliData api.CLIData, overrideBaseURL string, send func(tea.Msg)) NoPenaltyOnFail: step.NoPenaltyOnFail, }) - result := runCLICommand(*step.CLICommand, variables) + result := runCLICommand(*step.CLICommand, variables, shell) result.JqOutputs = collectStdoutJqOutputs(*step.CLICommand, result) results[i].CLICommandResult = &result diff --git a/checks/runner_test.go b/checks/runner_test.go index 2dff631..8668f4d 100644 --- a/checks/runner_test.go +++ b/checks/runner_test.go @@ -1,9 +1,10 @@ package checks import ( + "maps" "net/http" "net/http/httptest" - "reflect" + "os/exec" "strings" "testing" @@ -48,7 +49,7 @@ func TestCLIChecksInterpolatesResolvedBaseURLInCommands(t *testing.T) { }, }}, } - results, err := CLIChecks(cliData, tt.overrideBaseURL, func(tea.Msg) {}) + results, err := CLIChecks(cliData, RunOptions{OverrideBaseURL: tt.overrideBaseURL}, func(tea.Msg) {}) if err != nil { t.Fatalf("CLIChecks() error = %v", err) } @@ -84,7 +85,7 @@ func TestCLIChecksUsesOverrideParameterForHTTPRequestPreview(t *testing.T) { }}, } var sent []tea.Msg - results, err := CLIChecks(cliData, server.URL+"/", func(msg tea.Msg) { + results, err := CLIChecks(cliData, RunOptions{OverrideBaseURL: server.URL + "/"}, func(msg tea.Msg) { sent = append(sent, msg) }) if err != nil { @@ -123,7 +124,7 @@ func TestCLIChecksReturnsManifestErrors(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := CLIChecks(tt.data, "", func(tea.Msg) {}) + _, err := CLIChecks(tt.data, RunOptions{}, func(tea.Msg) {}) if err == nil || !strings.Contains(err.Error(), tt.want) { t.Fatalf("CLIChecks() error = %v, want error containing %q", err, tt.want) } @@ -137,17 +138,12 @@ func TestApplySubmissionResultsMarksAllStepsAndTestsPassedWhenNoFailure(t *testi {HTTPRequest: &api.CLIStepHTTPRequest{Tests: []api.HTTPRequestTest{{}}}}, }} - got := applySubmissionResultsMessages(cliData, nil) - want := []tea.Msg{ - messages.ResolveStepMsg{Index: 0, Passed: boolPtr(true)}, - messages.ResolveTestMsg{StepIndex: 0, TestIndex: 0, Passed: boolPtr(true)}, - messages.ResolveTestMsg{StepIndex: 0, TestIndex: 1, Passed: boolPtr(true)}, - messages.ResolveStepMsg{Index: 1, Passed: boolPtr(true)}, - messages.ResolveTestMsg{StepIndex: 1, TestIndex: 0, Passed: boolPtr(true)}, + steps, tests := submissionStatuses(cliData, nil) + if !maps.Equal(steps, map[int]bool{0: true, 1: true}) { + t.Fatalf("step statuses = %v, want both steps passed", steps) } - - if !reflect.DeepEqual(got, want) { - t.Fatalf("messages = %#v, want %#v", got, want) + if !maps.Equal(tests, map[[2]int]bool{{0, 0}: true, {0, 1}: true, {1, 0}: true}) { + t.Fatalf("test statuses = %v, want all three tests passed", tests) } } @@ -159,28 +155,83 @@ func TestApplySubmissionResultsStopsAfterFailedCLITest(t *testing.T) { }} failure := &api.StructuredErrCLI{FailedStepIndex: 1, FailedTestIndex: 1} - got := applySubmissionResultsMessages(cliData, failure) - want := []tea.Msg{ - messages.ResolveStepMsg{Index: 0, Passed: boolPtr(true)}, - messages.ResolveTestMsg{StepIndex: 0, TestIndex: 0, Passed: boolPtr(true)}, - messages.ResolveStepMsg{Index: 1, Passed: boolPtr(false)}, - messages.ResolveTestMsg{StepIndex: 1, TestIndex: 0, Passed: boolPtr(true)}, - messages.ResolveTestMsg{StepIndex: 1, TestIndex: 1, Passed: boolPtr(false)}, + steps, tests := submissionStatuses(cliData, failure) + if !maps.Equal(steps, map[int]bool{0: true, 1: false}) { + t.Fatalf("step statuses = %v, want first passed, second failed, third unresolved", steps) } - - if !reflect.DeepEqual(got, want) { - t.Fatalf("messages = %#v, want %#v", got, want) + if !maps.Equal(tests, map[[2]int]bool{{0, 0}: true, {1, 0}: true, {1, 1}: false}) { + t.Fatalf("test statuses = %v, want tests before failure passed and later tests unresolved", tests) } } -func applySubmissionResultsMessages(cliData api.CLIData, failure *api.StructuredErrCLI) []tea.Msg { - var msgs []tea.Msg - ApplySubmissionResults(cliData, failure, func(msg tea.Msg) { - msgs = append(msgs, msg) +func submissionStatuses(data api.CLIData, failure *api.StructuredErrCLI) (map[int]bool, map[[2]int]bool) { + steps := map[int]bool{} + tests := map[[2]int]bool{} + ApplySubmissionResults(data, failure, func(msg tea.Msg) { + switch msg := msg.(type) { + case messages.ResolveStepMsg: + if msg.Passed != nil { + steps[msg.Index] = *msg.Passed + } + case messages.ResolveTestMsg: + if msg.Passed != nil { + tests[[2]int{msg.StepIndex, msg.TestIndex}] = *msg.Passed + } + } }) - return msgs + return steps, tests +} + +func TestCLIChecksExplicitShell(t *testing.T) { + for _, tt := range []struct { + shell string + command string + }{ + {shell: "sh", command: "printf '%s' 'hello from shell'"}, + {shell: "pwsh", command: "Write-Output ('hello from ' + 'shell')"}, + } { + t.Run(tt.shell, func(t *testing.T) { + if _, err := exec.LookPath(tt.shell); err != nil { + t.Skipf("%s is not installed: %v", tt.shell, err) + } + data := api.CLIData{Steps: []api.CLIStep{{ + CLICommand: &api.CLIStepCLICommand{Command: tt.command}, + }}} + results, err := CLIChecks(data, RunOptions{Shell: tt.shell}, func(tea.Msg) {}) + if err != nil { + t.Fatal(err) + } + got := results[0].CLICommandResult + if got.Err != "" || got.ExitCode != 0 || got.Stdout != "hello from shell" { + t.Fatalf("unexpected shell result: %#v", got) + } + }) + } } -func boolPtr(v bool) *bool { - return &v +func TestCLIChecksRejectsShellBeforeRunningSteps(t *testing.T) { + for _, tt := range []struct { + shell string + want string + }{ + {shell: "bash", want: "unsupported shell"}, + {shell: "powershell", want: "unsupported shell"}, + {shell: "sh", want: "is unavailable"}, + {shell: "pwsh", want: "is unavailable"}, + } { + t.Run(tt.shell, func(t *testing.T) { + t.Setenv("PATH", t.TempDir()) + data := api.CLIData{Steps: []api.CLIStep{{ + HTTPRequest: &api.CLIStepHTTPRequest{}, + }, { + CLICommand: &api.CLIStepCLICommand{Command: "echo should not run"}, + }}} + _, err := CLIChecks(data, RunOptions{Shell: tt.shell}, func(tea.Msg) { + t.Fatal("step started before shell validation") + }) + if err == nil || !strings.Contains(err.Error(), tt.want) { + t.Fatalf("error = %v, want %q", err, tt.want) + } + }) + } } diff --git a/cmd/localtest.go b/cmd/localtest.go index dff8f97..18c9cb4 100644 --- a/cmd/localtest.go +++ b/cmd/localtest.go @@ -18,14 +18,17 @@ import ( func init() { rootCmd.AddCommand(localTestCmd) + localTestCmd.Flags().Bool("ignore-os", false, "skip the lesson's allowed operating systems check") + localTestCmd.Flags().String("shell", "", "shell for lesson commands: sh or pwsh (default depends on host OS)") localTestCmd.Flags().BoolVarP(&verboseOutput, "verbose", "v", false, "show detailed final output for every step") } var localTestCmd = &cobra.Command{ - Use: "local-test PATH", - Args: cobra.ExactArgs(1), - Hidden: true, - RunE: localTestHandler, + Use: "local-test PATH", + Args: cobra.ExactArgs(1), + Hidden: true, + RunE: localTestHandler, + Example: " bootdev local-test ./cli.yaml --ignore-os --shell pwsh", } func localTestHandler(cmd *cobra.Command, args []string) error { @@ -35,7 +38,17 @@ func localTestHandler(cmd *cobra.Command, args []string) error { if err != nil { return err } - if err := validateAllowedOS(data); err != nil { + ignoreOS, err := cmd.Flags().GetBool("ignore-os") + if err != nil { + return err + } + if !ignoreOS { + if err := validateAllowedOS(data); err != nil { + return err + } + } + shell, err := cmd.Flags().GetString("shell") + if err != nil { return err } @@ -51,7 +64,7 @@ func localTestHandler(cmd *cobra.Command, args []string) error { finish(submissionEvent) }() - cliResults, err := checks.CLIChecks(data, overrideBaseURL, send) + cliResults, err := checks.CLIChecks(data, checks.RunOptions{OverrideBaseURL: overrideBaseURL, Shell: shell}, send) if err != nil { return err } diff --git a/cmd/localtest_test.go b/cmd/localtest_test.go index 913eff9..1fd6973 100644 --- a/cmd/localtest_test.go +++ b/cmd/localtest_test.go @@ -3,25 +3,17 @@ package cmd import ( "os" "path/filepath" + "strings" "testing" - api "github.com/bootdotdev/bootdev/client" + "github.com/spf13/cobra" ) func TestReadLocalCLIDataAcceptsLessonDirectory(t *testing.T) { dir := t.TempDir() - manifest := []byte(`allowedOperatingSystems: - - linux - - darwin -baseURLDefault: http://localhost:3000 -steps: - - description: Prints a greeting - cliCommand: + manifest := []byte(`steps: + - cliCommand: command: echo hello - tests: - - exitCode: 0 - - stdoutContainsAll: - - hello `) if err := os.WriteFile(filepath.Join(dir, "cli.yaml"), manifest, 0o600); err != nil { t.Fatalf("failed to write test manifest: %v", err) @@ -31,29 +23,26 @@ steps: if err != nil { t.Fatalf("readLocalCLIData() error = %v", err) } - if data.BaseURLDefault != "http://localhost:3000" { - t.Fatalf("BaseURLDefault = %q, want localhost default", data.BaseURLDefault) - } - if len(data.Steps) != 1 || data.Steps[0].CLICommand == nil { - t.Fatalf("expected one CLI command step, got %#v", data.Steps) - } - if data.Steps[0].Description != "Prints a greeting" { - t.Fatalf("Description = %q, want manifest description", data.Steps[0].Description) - } - if len(data.Steps[0].CLICommand.Tests[1].StdoutContainsAll) != 1 { - t.Fatalf("expected stdoutContainsAll test to load") + if len(data.Steps) != 1 || data.Steps[0].CLICommand == nil || data.Steps[0].CLICommand.Command != "echo hello" { + t.Fatalf("expected one command loaded from cli.yaml, got %#v", data.Steps) } } -func TestLocalTestFailureErrorIncludesStructuredContext(t *testing.T) { - err := localTestFailureError(&api.StructuredErrCLI{ - ErrorMessage: `expected stdout to contain "hello"`, - FailedStepIndex: 1, - FailedTestIndex: 2, - }) - - want := "local checks failed: step 2, test 3\nexpected stdout to contain \"hello\"" - if err == nil || err.Error() != want { - t.Fatalf("localTestFailureError() = %v, want %q", err, want) +func TestLocalTestShellDoesNotBypassOSValidation(t *testing.T) { + for _, allowedOS := range []string{"[]", "[unsupported-os]"} { + t.Run(allowedOS, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "cli.yaml") + manifest := "allowedOperatingSystems: " + allowedOS + "\nsteps:\n - cliCommand:\n command: echo hello\n" + if err := os.WriteFile(path, []byte(manifest), 0o600); err != nil { + t.Fatal(err) + } + command := &cobra.Command{} + command.Flags().Bool("ignore-os", false, "") + command.Flags().String("shell", "pwsh", "") + err := localTestHandler(command, []string{path}) + if err == nil || !strings.Contains(err.Error(), "operating system") { + t.Fatalf("error = %v, want OS validation error", err) + } + }) } } diff --git a/cmd/root_test.go b/cmd/root_test.go index c38fb59..d04610a 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -1,7 +1,6 @@ package cmd import ( - "context" "io" "os" "path/filepath" @@ -11,7 +10,6 @@ import ( api "github.com/bootdotdev/bootdev/client" "github.com/bootdotdev/bootdev/version" - "github.com/spf13/cobra" "github.com/spf13/viper" ) @@ -41,12 +39,6 @@ func TestSecureConfigFileRestrictsExistingFilePermissions(t *testing.T) { } } -func TestSecureConfigFileAllowsEmptyPath(t *testing.T) { - if err := secureConfigFile(""); err != nil { - t.Fatalf("secureConfigFile() error = %v", err) - } -} - func TestExecuteSkipsVersionLookupForHelp(t *testing.T) { originalFetch := fetchUpdateInfo originalOut := rootCmd.OutOrStdout() @@ -75,31 +67,6 @@ func TestExecuteSkipsVersionLookupForHelp(t *testing.T) { } } -func TestLoadVersionInfoPopulatesCommandContext(t *testing.T) { - originalFetch := fetchUpdateInfo - t.Cleanup(func() { - fetchUpdateInfo = originalFetch - }) - - fetchUpdateInfo = func(currentVersion string) version.VersionInfo { - return version.VersionInfo{ - CurrentVersion: currentVersion, - LatestVersion: "v1.2.4", - IsOutdated: true, - } - } - - info := version.VersionInfo{} - cmd := &cobra.Command{Version: "v1.2.3"} - cmd.SetContext(version.WithContext(context.Background(), &info)) - - loadVersionInfo(cmd, nil) - - if info.CurrentVersion != "v1.2.3" || info.LatestVersion != "v1.2.4" || !info.IsOutdated { - t.Fatalf("version context was not populated: %#v", info) - } -} - func TestRefreshCredentialsPersistsRotatedCredentials(t *testing.T) { const ( oldAccessToken = "old-access-token" diff --git a/cmd/submit.go b/cmd/submit.go index 0efde79..6aca1c2 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -112,7 +112,7 @@ func submissionHandler(cmd *cobra.Command, args []string) error { } }() - cliResults, err := checks.CLIChecks(data, overrideBaseURL, send) + cliResults, err := checks.CLIChecks(data, checks.RunOptions{OverrideBaseURL: overrideBaseURL}, send) if err != nil { return err } diff --git a/go.mod b/go.mod index 2720fa2..cdc47b7 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/charmbracelet/bubbles v0.21.0 github.com/charmbracelet/bubbletea v1.3.10 github.com/charmbracelet/lipgloss v1.1.0 + github.com/charmbracelet/x/ansi v0.11.4 github.com/goccy/go-json v0.10.5 github.com/itchyny/gojq v0.12.18 github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c @@ -19,7 +20,6 @@ require ( require ( github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect github.com/charmbracelet/colorprofile v0.4.1 // indirect - github.com/charmbracelet/x/ansi v0.11.4 // indirect github.com/charmbracelet/x/cellbuf v0.0.14 // indirect github.com/charmbracelet/x/term v0.2.2 // indirect github.com/clipperhouse/displaywidth v0.7.0 // indirect diff --git a/main_test.go b/main_test.go new file mode 100644 index 0000000..316e1e9 --- /dev/null +++ b/main_test.go @@ -0,0 +1,85 @@ +package main + +import ( + "bytes" + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + "time" +) + +func TestLocalTestRedirectedOutput(t *testing.T) { + dir := t.TempDir() + binary := filepath.Join(dir, "bootdev.exe") + if output, err := exec.Command("go", "build", "-o", binary, ".").CombinedOutput(); err != nil { + t.Fatalf("build CLI: %v\n%s", err, output) + } + config := filepath.Join(dir, "config.yaml") + if err := os.WriteFile(config, []byte("{}\n"), 0o600); err != nil { + t.Fatal(err) + } + + command := `printf '\033[31mstdout-marker\033[0m\n'; printf 'stderr-marker\n' >&2` + if runtime.GOOS == "windows" { + command = `[Console]::Out.WriteLine([char]27 + '[31mstdout-marker' + [char]27 + '[0m'); [Console]::Error.WriteLine('stderr-marker')` + } + for _, tt := range []struct { + name string + fail bool + verbose bool + }{ + {"pass verbose", false, true}, + {"fail verbose", true, true}, + {"pass compact", false, false}, + {"fail compact", true, false}, + } { + t.Run(tt.name, func(t *testing.T) { + wantExit := 0 + if tt.fail { + wantExit = 1 + } + manifest := filepath.Join(t.TempDir(), "cli.yaml") + data := fmt.Sprintf("allowedOperatingSystems: [%s]\nsteps:\n - description: Output check\n cliCommand:\n command: %s\n tests:\n - exitCode: %d\n", runtime.GOOS, command, wantExit) + if err := os.WriteFile(manifest, []byte(data), 0o600); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + args := []string{"--config", config, "local-test", manifest} + if tt.verbose { + args = append(args, "--verbose") + } + cmd := exec.CommandContext(ctx, binary, args...) + var stdout, stderr bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, &stderr + err := cmd.Run() + if cmd.ProcessState == nil || cmd.ProcessState.ExitCode() != wantExit { + t.Fatalf("exit status: %v; stdout: %s; stderr: %s", err, &stdout, &stderr) + } + output := stdout.String() + if strings.Contains(output+stderr.String(), "\x1b") || strings.Contains(stderr.String(), "TTY") { + t.Fatalf("terminal output leaked: stdout=%q stderr=%q", output, stderr.String()) + } + if strings.Count(output, "Output check") != 1 { + t.Fatalf("expected one final report: %s", output) + } + for _, diagnostic := range []string{"Command stdout:\n\nstdout-marker", "Command stderr:\n\nstderr-marker"} { + if got, want := strings.Contains(output, diagnostic), tt.verbose || tt.fail; got != want { + t.Fatalf("diagnostic %q present = %t, want %t; output: %s", diagnostic, got, want, output) + } + } + if tt.fail { + if !strings.Contains(stderr.String(), "local checks failed") { + t.Fatalf("missing failure error: %s", &stderr) + } + } else if stderr.Len() != 0 || !strings.Contains(output, "All tests passed!") { + t.Fatalf("unexpected success output: stdout=%q stderr=%q", output, stderr.String()) + } + }) + } +} diff --git a/render/render.go b/render/render.go index 29a2aab..672237c 100644 --- a/render/render.go +++ b/render/render.go @@ -9,7 +9,9 @@ import ( "github.com/bootdotdev/bootdev/messages" tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" "github.com/spf13/viper" + "golang.org/x/term" ) var ( @@ -107,7 +109,12 @@ func (m rootModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { func StartRenderer(isSubmit bool, verbose bool, showOmitLessonIDTip bool) (func(tea.Msg), func(api.LessonSubmissionEvent)) { m := initModel(isSubmit, verbose) m.showOmitLessonIDTip = showOmitLessonIDTip - p := tea.NewProgram(m, tea.WithoutSignalHandler()) + interactive := term.IsTerminal(int(os.Stdin.Fd())) && term.IsTerminal(int(os.Stdout.Fd())) + options := []tea.ProgramOption{tea.WithoutSignalHandler(), tea.WithInput(nil)} + if !interactive { + options = append(options, tea.WithoutRenderer()) + } + p := tea.NewProgram(m, options...) done := make(chan struct{}) go func() { @@ -117,7 +124,11 @@ func StartRenderer(isSubmit bool, verbose bool, showOmitLessonIDTip bool) (func( } else if r, ok := model.(rootModel); ok { r.clear = false r.finalized = true - fmt.Fprint(os.Stdout, r.View()) + output := r.View() + if !interactive { + output = ansi.Strip(output) + } + fmt.Fprint(os.Stdout, output) } }() diff --git a/render/view_test.go b/render/view_test.go index 1ff6718..6703506 100644 --- a/render/view_test.go +++ b/render/view_test.go @@ -171,51 +171,6 @@ func TestSystemErrorViewDoesNotShowStepsAsPassed(t *testing.T) { } } -func TestOmitLessonIDTipOnlyAppearsAfterExplicitSuccessfulSubmission(t *testing.T) { - tests := []struct { - name string - isSubmit bool - showTip bool - result api.VerificationResultSlug - wantTip bool - }{ - {name: "explicit successful submission", isSubmit: true, showTip: true, result: api.VerificationResultSlugSuccess, wantTip: true}, - {name: "UUID-less successful submission", isSubmit: true, result: api.VerificationResultSlugSuccess}, - {name: "ordinary run with UUID", showTip: true, result: api.VerificationResultSlugSuccess}, - {name: "failed submission", isSubmit: true, showTip: true, result: api.VerificationResultSlugFailure}, - {name: "noop submission", isSubmit: true, showTip: true, result: api.VerificationResultSlugNoop}, - {name: "canceled or pre-submission error", isSubmit: true, showTip: true}, - {name: "system-error submission", isSubmit: true, showTip: true, result: api.VerificationResultSlugSystemError}, - } - - const tip = "Tip: When you reach your next CLI lesson, you can skip copying its ID:\n bootdev run" - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - m := initModel(tt.isSubmit, false) - m.finalized = true - m.showOmitLessonIDTip = tt.showTip - m.result = tt.result - if tt.result == api.VerificationResultSlugFailure || tt.result == api.VerificationResultSlugNoop { - m.failure = &api.StructuredErrCLI{FailedStepIndex: 0, ErrorMessage: "failed"} - } - - view := m.View() - if got := strings.Contains(view, tip); got != tt.wantTip { - t.Fatalf("tip present = %v, want %v\n%s", got, tt.wantTip, view) - } - if tt.wantTip { - if strings.Contains(view, "detect") { - t.Errorf("tip should not explain automatic detection\n%s", view) - } - browserInstruction := "Return to your browser to continue with the next lesson." - if !strings.Contains(view, browserInstruction) || strings.Index(view, browserInstruction) > strings.Index(view, tip) { - t.Fatalf("browser instruction must appear before tip\n%s", view) - } - } - }) - } -} - func TestStartStepFallsBackToTechnicalDescription(t *testing.T) { m := initModel(true, false) updated, _ := m.Update(messages.StartStepMsg{CMD: "go test ./..."})