From bf2a584788b20fd5cc69f2143454cf45f3556402 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Wed, 20 Sep 2023 01:00:34 +0200 Subject: [PATCH 1/3] Signal errors, context handler --- actors.go | 74 ++++++++++++++++++++++++++++++++++++++++++++------ actors_test.go | 59 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 9 deletions(-) create mode 100644 actors_test.go diff --git a/actors.go b/actors.go index d1bc754..10d50e2 100644 --- a/actors.go +++ b/actors.go @@ -2,23 +2,41 @@ package run import ( "context" + "errors" "fmt" "os" "os/signal" ) +// ContextHandler returns an actor, i.e. an execute and interrupt func, that +// terminates when the provided context is canceled. +func ContextHandler(ctx context.Context) (execute func() error, interrupt func(error)) { + ctx, cancel := context.WithCancel(ctx) + return func() error { + <-ctx.Done() + return ctx.Err() + }, func(error) { + cancel() + } +} + // SignalHandler returns an actor, i.e. an execute and interrupt func, that -// terminates with SignalError when the process receives one of the provided -// signals, or the parent context is canceled. +// terminates with ErrSignal when the process receives one of the provided +// signals, or with ctx.Error() when the parent context is canceled. If no +// signals are provided, the actor will terminate on any signal, per +// [signal.Notify]. func SignalHandler(ctx context.Context, signals ...os.Signal) (execute func() error, interrupt func(error)) { ctx, cancel := context.WithCancel(ctx) return func() error { - c := make(chan os.Signal, 1) - signal.Notify(c, signals...) - defer signal.Stop(c) + testc := getTestSigChan(ctx) + sigc := make(chan os.Signal, 1) + signal.Notify(sigc, signals...) + defer signal.Stop(sigc) select { - case sig := <-c: - return SignalError{Signal: sig} + case sig := <-testc: + return &SignalError{Signal: sig} + case sig := <-sigc: + return &SignalError{Signal: sig} case <-ctx.Done(): return ctx.Err() } @@ -27,13 +45,51 @@ func SignalHandler(ctx context.Context, signals ...os.Signal) (execute func() er } } -// SignalError is returned by the signal handler's execute function -// when it terminates due to a received signal. +type testSigChanKey struct{} + +func getTestSigChan(ctx context.Context) <-chan os.Signal { + return ctx.Value(testSigChanKey{}).(<-chan os.Signal) // can be nil +} + +func putTestSigChan(ctx context.Context, c <-chan os.Signal) context.Context { + return context.WithValue(ctx, testSigChanKey{}, c) +} + +// SignalError is returned by the signal handler's execute function when it +// terminates due to a received signal. +// +// SignalError has a design error that impacts comparison with errors.As. +// Callers should prefer using errors.Is(err, ErrSignal) to check for signal +// errors, and should only use errors.As in the rare case that they need to +// program against the specific os.Signal value. type SignalError struct { Signal os.Signal } // Error implements the error interface. +// +// It was a design error to define this method on a value receiver rather than a +// pointer receiver. For compatibility reasons it won't be changed. func (e SignalError) Error() string { return fmt.Sprintf("received signal %s", e.Signal) } + +// Is addresses a design error in the SignalError type, so that errors.Is with +// ErrSignal will return true. +func (e SignalError) Is(err error) bool { + return errors.Is(err, ErrSignal) +} + +// As fixes a design error in the SignalError type, so that errors.As with the +// literal `&SignalError{}` will return true. +func (e SignalError) As(target interface{}) bool { + switch target.(type) { + case *SignalError, SignalError: + return true + default: + return false + } +} + +// ErrSignal is returned by SignalHandler when a signal triggers termination. +var ErrSignal = errors.New("signal error") diff --git a/actors_test.go b/actors_test.go new file mode 100644 index 0000000..bbb5efa --- /dev/null +++ b/actors_test.go @@ -0,0 +1,59 @@ +package run + +import ( + "context" + "errors" + "os" + "testing" + "time" +) + +func TestContextHandler(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + var rg Group + rg.Add(ContextHandler(ctx)) + errc := make(chan error, 1) + go func() { errc <- rg.Run() }() + cancel() + select { + case err := <-errc: + if want, have := context.Canceled, err; !errors.Is(have, want) { + t.Errorf("error: want %v, have %v", want, have) + } + case <-time.After(time.Second): + t.Errorf("timeout waiting for error after cancel") + } +} + +func TestSignalError(t *testing.T) { + testc := make(chan os.Signal, 1) + ctx := putTestSigChan(context.Background(), testc) + + var rg Group + rg.Add(SignalHandler(ctx, os.Interrupt)) + testc <- os.Interrupt + err := rg.Run() + + var sigerr *SignalError + if want, have := true, errors.As(err, &sigerr); want != have { + t.Errorf("errors.As(err, &sigerr): want %v, have %v", want, have) + } + + if sigerr != nil { + if want, have := os.Interrupt, sigerr.Signal; want != have { + t.Errorf("sigerr.Signal: want %v, have %v", want, have) + } + } + + if sigerr := &(SignalError{}); !errors.As(err, &sigerr) { + t.Errorf("errors.As(err, ): failed") + } + + if want, have := true, errors.As(err, &(SignalError{})); want != have { + t.Errorf("errors.As(err, &(SignalError{})): want %v, have %v", want, have) + } + + if want, have := true, errors.Is(err, ErrSignal); want != have { + t.Errorf("errors.Is(err, ErrSignal): want %v, have %v", want, have) + } +} From 3a6f5761f3731fd5d9ee47befbd4f8074a01994f Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Wed, 20 Sep 2023 01:01:08 +0200 Subject: [PATCH 2/3] go1.20 --- .github/workflows/test.yml | 58 +++++++++++++++++++------------------- go.mod | 2 +- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 177bd69..b8fb396 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -4,35 +4,35 @@ jobs: test: strategy: matrix: - go-version: [1.13.x] + go-version: [1.20.x] platform: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.platform }} steps: - - name: Install Go - uses: actions/setup-go@v1 - with: - go-version: ${{ matrix.go-version }} - - name: Install staticcheck - run: pwd && cd .. && go get -v -u honnef.co/go/tools/cmd/staticcheck && cd - - shell: bash - - name: Install golint - run: pwd && cd .. && go get -v -u golang.org/x/lint/golint && cd - - shell: bash - - name: Update PATH - # https://github.com/actions/setup-go/issues/12#issuecomment-524631719 - run: echo "##[add-path]$(go env GOPATH)/bin" - shell: bash - - name: Checkout code - uses: actions/checkout@v1 - - name: Fmt - if: matrix.platform != 'windows-latest' # :( - run: "F=$(gofmt -l .) ; if [[ $F ]] ; then echo $F ; exit 1 ; fi" - shell: bash - - name: Vet - run: go vet ./... - - name: Staticcheck - run: staticcheck ./... - - name: Lint - run: golint ./... - - name: Test - run: go test -race ./... + - name: Install Go + uses: actions/setup-go@v1 + with: + go-version: ${{ matrix.go-version }} + - name: Install staticcheck + run: pwd && cd .. && go get -v -u honnef.co/go/tools/cmd/staticcheck && cd - + shell: bash + - name: Install golint + run: pwd && cd .. && go get -v -u golang.org/x/lint/golint && cd - + shell: bash + - name: Update PATH + # https://github.com/actions/setup-go/issues/12#issuecomment-524631719 + run: echo "##[add-path]$(go env GOPATH)/bin" + shell: bash + - name: Checkout code + uses: actions/checkout@v1 + - name: Fmt + if: matrix.platform != 'windows-latest' # :( + run: "F=$(gofmt -l .) ; if [[ $F ]] ; then echo $F ; exit 1 ; fi" + shell: bash + - name: Vet + run: go vet ./... + - name: Staticcheck + run: staticcheck ./... + - name: Lint + run: golint ./... + - name: Test + run: go test -race ./... diff --git a/go.mod b/go.mod index b129723..caa7fc4 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/oklog/run -go 1.13 +go 1.20 From 57d4d9b9a794b3c47973fdd41208b2c7a69474a5 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Wed, 20 Sep 2023 01:04:30 +0200 Subject: [PATCH 3/3] Update GitHub Action --- .github/workflows/test.yaml | 74 +++++++++++++++++++++++++++++++++++++ .github/workflows/test.yml | 38 ------------------- 2 files changed, 74 insertions(+), 38 deletions(-) create mode 100644 .github/workflows/test.yaml delete mode 100644 .github/workflows/test.yml diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml new file mode 100644 index 0000000..fc850dc --- /dev/null +++ b/.github/workflows/test.yaml @@ -0,0 +1,74 @@ +name: test +on: + pull_request: + types: [opened, synchronize] + push: + branches: [main] + schedule: + - cron: "0 12 1 * *" # first day of the month at 12:00 + +jobs: + test: + strategy: + matrix: + platform: [ubuntu-latest, macos-latest, windows-latest] + + runs-on: ${{ matrix.platform }} + + defaults: + run: + shell: bash + + steps: + - name: Install Go + uses: actions/setup-go@v3 + with: + go-version: 1.20.x + + - name: Check out repo + uses: actions/checkout@v3 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - name: Prepare cache + id: cache + run: | + echo "GOCACHE=$(go env GOCACHE)" >> $GITHUB_OUTPUT + echo "GOMODCACHE=$(go env GOMODCACHE)" >> $GITHUB_OUTPUT + echo "GOVERSION=$(go env GOVERSION)" >> $GITHUB_OUTPUT + mkdir -p $(go env GOCACHE) || true + mkdir -p $(go env GOMODCACHE) || true + + - name: Cache + uses: actions/cache@v3 + with: + path: | + ${{ steps.cache.outputs.GOCACHE }} + ${{ steps.cache.outputs.GOMODCACHE }} + key: test.1-${{ runner.os }}-${{ steps.cache.outputs.GOVERSION }}-${{ hashFiles('**/go.mod') }} + restore-keys: | + test.1-${{ runner.os }}-${{ steps.cache.outputs.GOVERSION }}-${{ hashFiles('**/go.mod') }} + test.1-${{ runner.os }}-${{ steps.cache.outputs.GOVERSION }}- + test.1-${{ runner.os }}- + + - name: Install tools + run: | + go install honnef.co/go/tools/cmd/staticcheck@latest + go install mvdan.cc/gofumpt@latest + go install github.com/mgechev/revive@latest + + - name: Run gofmt + if: matrix.platform != 'windows-latest' # :< + run: diff <(gofmt -d . 2>/dev/null) <(printf '') + + - name: Run go vet + run: go vet ./... + + - name: Run staticcheck + run: staticcheck ./... + + - name: Run gofumpt + run: gofumpt -d -e -l . + + - name: Run go test + run: go test -v -race ./... diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml deleted file mode 100644 index b8fb396..0000000 --- a/.github/workflows/test.yml +++ /dev/null @@ -1,38 +0,0 @@ -on: push -name: Test -jobs: - test: - strategy: - matrix: - go-version: [1.20.x] - platform: [ubuntu-latest, macos-latest, windows-latest] - runs-on: ${{ matrix.platform }} - steps: - - name: Install Go - uses: actions/setup-go@v1 - with: - go-version: ${{ matrix.go-version }} - - name: Install staticcheck - run: pwd && cd .. && go get -v -u honnef.co/go/tools/cmd/staticcheck && cd - - shell: bash - - name: Install golint - run: pwd && cd .. && go get -v -u golang.org/x/lint/golint && cd - - shell: bash - - name: Update PATH - # https://github.com/actions/setup-go/issues/12#issuecomment-524631719 - run: echo "##[add-path]$(go env GOPATH)/bin" - shell: bash - - name: Checkout code - uses: actions/checkout@v1 - - name: Fmt - if: matrix.platform != 'windows-latest' # :( - run: "F=$(gofmt -l .) ; if [[ $F ]] ; then echo $F ; exit 1 ; fi" - shell: bash - - name: Vet - run: go vet ./... - - name: Staticcheck - run: staticcheck ./... - - name: Lint - run: golint ./... - - name: Test - run: go test -race ./...