From 4cd31f0d23729d78bf5ff43ea44da009a391afa2 Mon Sep 17 00:00:00 2001 From: Paul Kuruvilla Date: Fri, 24 May 2024 19:14:55 +0100 Subject: [PATCH 1/3] update failure case --- internal/stages_test.go | 7 ++++++ internal/test_helpers/failure/simple_shell.rb | 24 ------------------- internal/test_helpers/failure/your_shell.sh | 4 ---- .../fixtures/missing_command/wrong_output | 9 +++++++ .../wrong_output}/codecrafters.yml | 0 .../missing_command/wrong_output/main.py | 16 +++++++++++++ .../wrong_output/your_shell.sh | 8 +++++++ 7 files changed, 40 insertions(+), 28 deletions(-) delete mode 100755 internal/test_helpers/failure/simple_shell.rb delete mode 100755 internal/test_helpers/failure/your_shell.sh create mode 100644 internal/test_helpers/fixtures/missing_command/wrong_output rename internal/test_helpers/{failure => scenarios/missing_command/wrong_output}/codecrafters.yml (100%) create mode 100755 internal/test_helpers/scenarios/missing_command/wrong_output/main.py create mode 100755 internal/test_helpers/scenarios/missing_command/wrong_output/your_shell.sh diff --git a/internal/stages_test.go b/internal/stages_test.go index 1dd9cd36..a28d0c11 100644 --- a/internal/stages_test.go +++ b/internal/stages_test.go @@ -26,6 +26,13 @@ func TestStages(t *testing.T) { StdoutFixturePath: "./test_helpers/fixtures/navigation/pass", NormalizeOutputFunc: normalizeTesterOutput, }, + "missing_command_fail": { + UntilStageSlug: "cz2", + CodePath: "./test_helpers/scenarios/missing_command/wrong_output", + ExpectedExitCode: 1, + StdoutFixturePath: "./test_helpers/fixtures/missing_command/wrong_output", + NormalizeOutputFunc: normalizeTesterOutput, + }, } testerUtilsTesting.TestTesterOutput(t, testerDefinition, testCases) diff --git a/internal/test_helpers/failure/simple_shell.rb b/internal/test_helpers/failure/simple_shell.rb deleted file mode 100755 index 6f0dedd0..00000000 --- a/internal/test_helpers/failure/simple_shell.rb +++ /dev/null @@ -1,24 +0,0 @@ -# Case 1: No output -# $stdout.write " " - -# Case 2: Prompt but contains newline -# puts "$ " -# sleep 5 - -# # Case 3: Proper prompt -# $stdout.write("$ ") -# sleep 5 - -loop do - $stdout.write("$ ") - command, *_args = gets.chomp.split(" ") - - case command - when "exit" - break - else - puts "#{command}: command not found" - end -end - -puts "Goodbye!" diff --git a/internal/test_helpers/failure/your_shell.sh b/internal/test_helpers/failure/your_shell.sh deleted file mode 100755 index feef6891..00000000 --- a/internal/test_helpers/failure/your_shell.sh +++ /dev/null @@ -1,4 +0,0 @@ -#!/bin/sh -set -e -cd $(dirname "$0") -exec ruby simple_shell.rb diff --git a/internal/test_helpers/fixtures/missing_command/wrong_output b/internal/test_helpers/fixtures/missing_command/wrong_output new file mode 100644 index 00000000..0ed5c83d --- /dev/null +++ b/internal/test_helpers/fixtures/missing_command/wrong_output @@ -0,0 +1,9 @@ +Debug = true + +[stage-2] Running tests for Stage #2: cz2 +[stage-2] Running ./your_shell.sh +[your-program] $ +[stage-2] > nonexistent +[your-program] Command: nonexistent +[stage-2] EOF +[stage-2] Test failed diff --git a/internal/test_helpers/failure/codecrafters.yml b/internal/test_helpers/scenarios/missing_command/wrong_output/codecrafters.yml similarity index 100% rename from internal/test_helpers/failure/codecrafters.yml rename to internal/test_helpers/scenarios/missing_command/wrong_output/codecrafters.yml diff --git a/internal/test_helpers/scenarios/missing_command/wrong_output/main.py b/internal/test_helpers/scenarios/missing_command/wrong_output/main.py new file mode 100755 index 00000000..23b714b8 --- /dev/null +++ b/internal/test_helpers/scenarios/missing_command/wrong_output/main.py @@ -0,0 +1,16 @@ +import sys + + +def main(): + sys.stdout.write("$ ") + sys.stdout.flush() + + command = input() + + # Wrong output + sys.stdout.write(f"Command: {command}\n") + sys.stdout.flush() + + +if __name__ == "__main__": + main() diff --git a/internal/test_helpers/scenarios/missing_command/wrong_output/your_shell.sh b/internal/test_helpers/scenarios/missing_command/wrong_output/your_shell.sh new file mode 100755 index 00000000..118ddfad --- /dev/null +++ b/internal/test_helpers/scenarios/missing_command/wrong_output/your_shell.sh @@ -0,0 +1,8 @@ +#!/bin/sh +# +# DON'T EDIT THIS! +# +# CodeCrafters uses this file to test your code. Don't make any changes here! +# +# DON'T EDIT THIS! +exec python3 $(dirname "$0")/main.py "$@" From 16964c458511bfa37d762447708d48a7f6c80236 Mon Sep 17 00:00:00 2001 From: Paul Kuruvilla Date: Fri, 24 May 2024 19:27:43 +0100 Subject: [PATCH 2/3] Refactor shell_executable package and fix error handling in stage4.go The changes in the diff include refactoring the shell_executable package in the condition_reader/condition_reader.go file. The ReadBytesUntil and ReadBytesUntilTimeout methods now wrap the reader error to handle program exit errors properly. In the stage4.go file, the error handling for program exit in the testExit function has been updated to check for the ErrProgramExited error instead of specific error types. Additionally, there is a change in the fixtures/missing_command/wrong_output file, which shows that the program exited instead of reaching EOF. Commit message: "Refactor shell_executable package and fix error handling in stage4.go" --- internal/condition_reader/condition_reader.go | 2 +- internal/shell_executable/shell_executable.go | 29 +++++++++++++++++-- internal/stage4.go | 9 ++---- .../fixtures/missing_command/wrong_output | 2 +- 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/internal/condition_reader/condition_reader.go b/internal/condition_reader/condition_reader.go index 33c019b2..70a66f26 100644 --- a/internal/condition_reader/condition_reader.go +++ b/internal/condition_reader/condition_reader.go @@ -80,4 +80,4 @@ func (t *ConditionReader) ReadUntilTimeout(timeout time.Duration) ([]byte, error } return data, err -} +} \ No newline at end of file diff --git a/internal/shell_executable/shell_executable.go b/internal/shell_executable/shell_executable.go index 121832b5..3134ce48 100644 --- a/internal/shell_executable/shell_executable.go +++ b/internal/shell_executable/shell_executable.go @@ -1,11 +1,14 @@ package shell_executable import ( + "errors" "fmt" + "io" "os" "os/exec" "regexp" "strings" + "syscall" "time" "github.com/codecrafters-io/shell-tester/internal/condition_reader" @@ -18,6 +21,9 @@ import ( // ErrConditionNotMet is re-exported from condition_reader for convenience var ErrConditionNotMet = condition_reader.ErrConditionNotMet +// ErrProgramExited is returned when the program exits +var ErrProgramExited = errors.New("Program exited") + type ShellExecutable struct { executable *executable.Executable logger *logger.Logger @@ -70,11 +76,21 @@ func (b *ShellExecutable) LogOutput(output []byte) { } func (b *ShellExecutable) ReadBytesUntil(condition func([]byte) bool) ([]byte, error) { - return b.ptyReader.ReadUntilCondition(condition) + readBytes, err := b.ptyReader.ReadUntilCondition(condition) + if err != nil { + return readBytes, wrapReaderError(err) + } + + return readBytes, nil } func (b *ShellExecutable) ReadBytesUntilTimeout(timeout time.Duration) ([]byte, error) { - return b.ptyReader.ReadUntilTimeout(timeout) + readBytes, err := b.ptyReader.ReadUntilTimeout(timeout) + if err != nil { + return readBytes, wrapReaderError(err) + } + + return readBytes, nil } func (b *ShellExecutable) SendCommand(command string) error { @@ -159,3 +175,12 @@ func StripANSI(data []byte) []byte { return re.ReplaceAll(data, []byte("")) } + +func wrapReaderError(readerErr error) error { + // Linux returns syscall.EIO when the process is killed, MacOS returns io.EOF + if errors.Is(readerErr, io.EOF) || errors.Is(readerErr, syscall.EIO) { + return ErrProgramExited + } + + return readerErr +} diff --git a/internal/stage4.go b/internal/stage4.go index bc621f33..17a08211 100644 --- a/internal/stage4.go +++ b/internal/stage4.go @@ -1,12 +1,9 @@ package internal import ( - "errors" "fmt" - "io" "regexp" "strings" - "syscall" "time" "github.com/codecrafters-io/shell-tester/internal/shell_executable" @@ -54,13 +51,11 @@ func testExit(stageHarness *test_case_harness.TestCaseHarness) error { } // We're expecting EOF since the program should've terminated - // HACK: We also allow syscall.EIO since that's what we get on Linux when the process is killed - // read /dev/ptmx: input/output error *fs.PathError - if !errors.Is(readErr, io.EOF) && !errors.Is(readErr, syscall.EIO) { + if readErr != shell_executable.ErrProgramExited { if readErr == nil { return fmt.Errorf("Expected program to exit with 0 exit code, program is still running.") } else { - // TODO: Other than EOF, what other errors could we get? Are they user errors or internal errors? + // TODO: Other than ErrProgramExited, what other errors could we get? Are they user errors or internal errors? return fmt.Errorf("Error reading output: %v", readErr) } } diff --git a/internal/test_helpers/fixtures/missing_command/wrong_output b/internal/test_helpers/fixtures/missing_command/wrong_output index 0ed5c83d..74257b8c 100644 --- a/internal/test_helpers/fixtures/missing_command/wrong_output +++ b/internal/test_helpers/fixtures/missing_command/wrong_output @@ -5,5 +5,5 @@ Debug = true [your-program] $ [stage-2] > nonexistent [your-program] Command: nonexistent -[stage-2] EOF +[stage-2] Program exited [stage-2] Test failed From e281c8c268de8147df86dc7a52d6c5c0496226f2 Mon Sep 17 00:00:00 2001 From: Paul Kuruvilla Date: Fri, 24 May 2024 19:29:10 +0100 Subject: [PATCH 3/3] Commit message: "Fix error handling and add missing command test case" --- internal/test_cases/regex_test_case.go | 3 ++- internal/test_helpers/fixtures/missing_command/wrong_output | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/test_cases/regex_test_case.go b/internal/test_cases/regex_test_case.go index ac3643b1..a6f158da 100644 --- a/internal/test_cases/regex_test_case.go +++ b/internal/test_cases/regex_test_case.go @@ -52,10 +52,11 @@ func (t RegexTestCase) Run(shell *shell_executable.ShellExecutable, logger *logg } if err != nil { - if errors.Is(err, shell_executable.ErrConditionNotMet) { + if errors.Is(err, shell_executable.ErrConditionNotMet) || errors.Is(err, shell_executable.ErrProgramExited) { logger.Errorf("Expected output to %s, got %q", t.ExpectedPatternExplanation, string(shell_executable.StripANSI(output))) } + // TODO: Think about this, what other errors could be there other than ErrConditionNotMet and ErrProgramExited? return err } diff --git a/internal/test_helpers/fixtures/missing_command/wrong_output b/internal/test_helpers/fixtures/missing_command/wrong_output index 74257b8c..656d3355 100644 --- a/internal/test_helpers/fixtures/missing_command/wrong_output +++ b/internal/test_helpers/fixtures/missing_command/wrong_output @@ -5,5 +5,6 @@ Debug = true [your-program] $ [stage-2] > nonexistent [your-program] Command: nonexistent +[stage-2] Expected output to contain "nonexistent: command not found", got "Command: nonexistent\r\n" [stage-2] Program exited [stage-2] Test failed