Skip to content

Commit

Permalink
Add explanatory comment & some test cases
Browse files Browse the repository at this point in the history
- Add a comment about why we test for error after processing data from
  readLine.
- Add some test cases. Daniel suggested four, three of which already
  pass without this PR. I left them in but commented-out, in case
  somebody wants to look at them.
- Add a test for cancelling an already-running read.
  • Loading branch information
theclapp committed Mar 20, 2024
1 parent c3175c5 commit 72db5a6
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 5 deletions.
8 changes: 3 additions & 5 deletions interp/builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"context"
"errors"
"fmt"
"io"
"os"
"path/filepath"
"strconv"
Expand Down Expand Up @@ -604,6 +603,8 @@ func (r *Runner) builtinCode(ctx context.Context, pos syntax.Pos, name string, a
r.setVarString(name, val)
}

// We can get data back from readLine and an error at the same time, so
// check err after we process the data.
if err != nil {
return 1
}
Expand Down Expand Up @@ -966,11 +967,8 @@ func (r *Runner) readLine(ctx context.Context, raw bool) ([]byte, error) {
esc = false
}
}
if err == io.EOF && len(line) > 0 {
return line, nil
}
if err != nil {
return nil, err
return line, err
}
}
}
Expand Down
64 changes: 64 additions & 0 deletions interp/interp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2982,6 +2982,27 @@ done <<< 2`,
"mapfile -t butter <<EOF\na\nb\nc\nEOF\n" + `for x in "${butter[@]}"; do echo "$x"; done`,
"a\nb\nc\n",
},
// read & EOF
// // newline
// {
// `a=a; echo | read a; echo -n "$a"`,
// "",
// },
// empty input
{
`a=b; read a < /dev/null; echo -n "$a"`,
"",
},
// // some string and newline
// {
// "a=c; echo x | read a; echo -n $a",
// "x",
// },
// // some string and EOF
// {
// "a=d; echo -n y | read a; echo -n $a",
// "y",
// },
}

var runTestsUnix = []runTest{
Expand Down Expand Up @@ -3943,6 +3964,49 @@ func TestRunnerContext(t *testing.T) {
}
}

func TestCancelreader(t *testing.T) {
t.Parallel()

p := syntax.NewParser()
file := parse(t, p, "read x")
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
// Make the linter happy, even though we deliberately wait for the
// timeout.
defer cancel()

var r *interp.Runner
if runtime.GOOS == "windows" {
// On Windows, the cancelreader only works on stdin
r, _ = interp.New(interp.StdIO(os.Stdin, nil, nil))
} else {
siR, siW, err := os.Pipe()
if err != nil {
t.Fatalf("Error calling os.Pipe: %v", err)
}
defer func() {
siW.Close()
siR.Close()
}()
r, _ = interp.New(interp.StdIO(siR, nil, nil))
}
now := time.Now()
errChan := make(chan error)
go func() {
errChan <- r.Run(ctx, file)
}()

timeout := 500 * time.Millisecond
select {
case err := <-errChan:
if err == nil || err.Error() != "exit status 1" || ctx.Err() != context.DeadlineExceeded {
t.Fatalf("'read x' did not timeout correctly; err: %v, ctx.Err(): %v; dur: %v",
err, ctx.Err(), time.Since(now))
}
case <-time.After(timeout):
t.Fatalf("program was not killed in %s", timeout)
}
}

func TestRunnerAltNodes(t *testing.T) {
t.Parallel()

Expand Down

0 comments on commit 72db5a6

Please sign in to comment.