Skip to content

Commit

Permalink
ci: Fix -race, drop -v, fix broken test (#82)
Browse files Browse the repository at this point in the history
This change fixes our CI setup to actually enable data race detection.
The `NO_RACE` logic we had in place was always disabling data race
detection,
so we didn't catch a data race in one of the tests.

This fixes that. Example failure:
https://github.com/bracesdev/errtrace/actions/runs/7302696877/job/19901817884
<img width="505" alt="image"
src="https://github.com/bracesdev/errtrace/assets/41730/8d11b04f-b351-48da-a72e-1cee1cb1643e">

Data race detection is enabled only on amd64
because we run arm64 with qemu, and -race requires cgo.
Example failure from ubuntu arm64:

```
go test -coverprofile cover.unsafe.out -coverpkg ./... -race ./...
go: -race requires cgo
```

Lastly, this drops the `-v` from when we run tests.
The verbose output makes it harder to find the actual test failure.
Without `-v`, only failures will be printed.
  • Loading branch information
abhinav authored Jan 9, 2024
1 parent 3e960df commit 0110cc8
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 12 deletions.
15 changes: 13 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,24 @@ jobs:
if: matrix.arch != 'amd64' && matrix.arch != '386'
uses: docker/setup-qemu-action@v3

- name: Enable race detection
shell: bash
run: |
# Only amd64 support data-race detection in CI.
# qemu doesn't give us cgo, which is needed for arm64.
if [[ "$GOARCH" == amd64 ]]; then
echo "Enabling data-race detection."
else
echo "NO_RACE=1" >> "$GITHUB_ENV"
fi
env:
GOARCH: ${{ matrix.arch }}

- name: Test ${{ matrix.arch }}
run: make cover
shell: bash
env:
GOARCH: ${{ matrix.arch }}
# Only amd64/arm64 support RACE, disable it on all other architectures.
NO_RACE: ${{ (matrix.arch == 'amd64' || matrix.arch == 'arm64') && '' || '1' }}

- name: Coverage
uses: codecov/codecov-action@v3
Expand Down
8 changes: 4 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ GOLANGCI_LINT_ARGS ?=

.PHONY: test
test:
go test $(RACE) -v ./...
go test $(RACE) -tags safe -v ./...
go test $(RACE) ./...
go test $(RACE) -tags safe ./...
go test -gcflags='-l -N' ./... # disable optimizations/inlining

.PHONY: cover
cover:
go test -coverprofile cover.unsafe.out -coverpkg ./... $(RACE) -v ./...
go test -coverprofile cover.safe.out -coverpkg ./... $(RACE) -tags safe -v ./...
go test -coverprofile cover.unsafe.out -coverpkg ./... $(RACE) ./...
go test -coverprofile cover.safe.out -coverpkg ./... $(RACE) -tags safe ./...
go test ./... -gcflags='-l -N' ./... # disable optimizations/inlining

.PHONY: bench
Expand Down
6 changes: 3 additions & 3 deletions cmd/errtrace/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -579,13 +579,13 @@ func (cmd *mainCmd) readFile(r fileRequest) ([]byte, error) {
// wait a short time for the first read.
// If there's nothing, print a warning and continue waiting.
firstRead := make(chan struct{})
go func(firstRead <-chan struct{}) {
go func(firstRead <-chan struct{}, wait time.Duration) {
select {
case <-firstRead:
case <-time.After(_stdinWait):
case <-time.After(wait):
cmd.log.Println("reading from stdin; use '-h' for help")
}
}(firstRead)
}(firstRead, _stdinWait)

var buff bytes.Buffer
bs := make([]byte, 1024)
Expand Down
34 changes: 31 additions & 3 deletions cmd/errtrace/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -768,9 +768,8 @@ func TestStdinNoInputMessage(t *testing.T) {
t.Error(err)
}
}()
defer func() { <-done }()

var stderr bytes.Buffer
var stderr lockedBuffer
exitCode := (&mainCmd{
Stdin: stdin,
Stdout: io.Discard,
Expand All @@ -780,14 +779,43 @@ func TestStdinNoInputMessage(t *testing.T) {
if want := 0; exitCode != want {
t.Errorf("exit code = %d, want %d", exitCode, want)
}
<-done

// errtrace's goroutine may still be writing to stderr.
// Try a few times before giving up.
var gotStderr string
for i := 0; i < 10; i++ {
gotStderr = stderr.String()
if gotStderr != tt.wantStderr {
time.Sleep(100 * time.Millisecond)
continue
}
}

if want, got := tt.wantStderr, stderr.String(); got != want {
if want, got := tt.wantStderr, gotStderr; got != want {
t.Errorf("stderr = %q, want %q", got, want)
}
})
}
}

type lockedBuffer struct {
mu sync.RWMutex
buf bytes.Buffer
}

func (b *lockedBuffer) Write(p []byte) (int, error) {
b.mu.Lock()
defer b.mu.Unlock()
return errtrace.Wrap2(b.buf.Write(p))
}

func (b *lockedBuffer) String() string {
b.mu.RLock()
defer b.mu.RUnlock()
return b.buf.String()
}

func indent(s string) string {
return "\t" + strings.ReplaceAll(s, "\n", "\n\t")
}
Expand Down

0 comments on commit 0110cc8

Please sign in to comment.