Skip to content

Commit

Permalink
Simplify 32 bit CI workflow & lint, test on Go 1.19-1.22 (#45)
Browse files Browse the repository at this point in the history
A couple changes to simplify various workflows, plus explicitly testing
on the latest Go versions.

* Enable race on all supported platforms, explicitly set -race= flag
* Move linters-settings below linters list
* Reformat octal integers to latest gofmt style
* Fix comments to strictly follow godoc style
* Disable govet's fieldalignment memory size check on test files
* Test on Go 1.19-1.22, verify 32 bit field alignment on Linux with
latest Go
  • Loading branch information
JohnStarich authored Jun 23, 2024
1 parent 58eb4cd commit b05f4b7
Show file tree
Hide file tree
Showing 19 changed files with 205 additions and 200 deletions.
34 changes: 22 additions & 12 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,31 +12,41 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: 1.19.x
go-version: 1.21.x
- name: Lint
run: make lint

test:
strategy:
matrix:
platform: [ ubuntu-latest, windows-latest, macos-latest ]
version: [ v1.19.x ]
goarch: [ amd64, '386' ]
exclude:
- platform: macos-latest
goarch: '386'
- platform: windows-latest
goarch: '386'
platform:
- ubuntu-latest
go:
- 1.19.x
- 1.20.x
- 1.21.x
- 1.22.x
goarch:
- amd64
include:
- platform: ubuntu-latest
go: 1.21.x
goarch: '386' # Verify fieldalignment on 32 bit platforms.
- platform: macos-latest
go: 1.21.x
goarch: amd64
- platform: windows-latest
go: 1.21.x
goarch: amd64
runs-on: ${{ matrix.platform }}
steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: ${{ matrix.version }}
architecture: ${{ matrix.goarch }}
go-version: ${{ matrix.go }}
- name: Test
run: make test
env:
GOARCH: ${{ matrix.goarch }}
COVERALLS_TOKEN: ${{ secrets.GITHUB_TOKEN }}
COVERAGE_VERSION: '1.19'
COVERAGE_VERSION: '1.21'
15 changes: 10 additions & 5 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
linters-settings:
govet:
enable:
- fieldalignment

linters:
enable:
# Default linters, plus these:
Expand All @@ -16,13 +11,23 @@ linters:
- paralleltest
- revive

linters-settings:
govet:
enable:
- fieldalignment

issues:
exclude:
# Disable scopelint errors on table driven tests
- Using the variable on range scope `tc` in function literal
# Disable documenting fstest Test functions
- exported function Test\S* should have comment or be unexported
- comment on exported function Test\S* should be of the form
exclude-rules:
- path: '(.+)_test\.go|^fstest/' # Disable some lint failures on test files and packages.
linters:
- govet
text: 'fieldalignment: struct with .* bytes could be .*' # Govet's fieldalignment memory size check on table-driven test case types requires field reordering to improve performance, which can lower readability without a meaningful impact to non-test code.
include:
# Re-enable default excluded rules
- EXC0001
Expand Down
26 changes: 10 additions & 16 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
BROWSERTEST_VERSION = v0.7
BROWSERTEST_VERSION = v0.8
LINT_VERSION = 1.59.1
GO_BIN = $(shell printf '%s/bin' "$$(go env GOPATH)")
SHELL = bash
ENABLE_RACE = true

current_platform := $(shell go env GOOS)/$(shell go env GOARCH)
# Only use the race detector on supported architectures.
# Go's supported platforms pulled from 'go help build' under '-race'.
race_platforms := linux/amd64 freebsd/amd64 darwin/amd64 darwin/arm64 windows/amd64 linux/ppc64le linux/arm64
RACE_ENABLED = $(if $(findstring ${current_platform},${race_platforms}),true,false)

.PHONY: all
all: lint test
Expand All @@ -27,30 +32,19 @@ lint: lint-deps
test-deps:
@if [ ! -f "${GO_BIN}/go_js_wasm_exec" ]; then \
set -ex; \
go install github.com/agnivade/wasmbrowsertest@${BROWSERTEST_VERSION}; \
GOOS= GOARCH= go install github.com/agnivade/wasmbrowsertest@${BROWSERTEST_VERSION}; \
ln -s "${GO_BIN}/wasmbrowsertest" "${GO_BIN}/go_js_wasm_exec"; \
fi
@go install github.com/mattn/[email protected]

# only use the race detector on supported architectures
race_goarches := 'amd64' 'arm64'
ifeq (,$(findstring '$(GOARCH)',$(race_goarches)))
TEST_ARGS=
export CGO_ENABLED=0
else
TEST_ARGS=-race
# the race detector needs cgo (at least on Linux and Windows, and macOS until Go 1.20)
export CGO_ENABLED=1
endif

.PHONY: test
test: test-deps
go test . # Run library-level checks first, for more helpful build tag failure messages.
go test $(TEST_ARGS) -coverprofile=native-cover.out ./...
go test -race=${RACE_ENABLED} -coverprofile=native-cover.out ./...
if [[ "$$CI" != true || $$(uname -s) == Linux ]]; then \
set -ex; \
GOOS=js GOARCH=wasm go test -coverprofile=js-cover.out -covermode=atomic ./...; \
cd examples && go test $(TEST_ARGS) ./...; \
cd examples && go test -race=${RACE_ENABLED} ./...; \
fi
{ echo 'mode: atomic'; cat *-cover.out | grep -v '^mode:'; } > cover.out && rm *-cover.out
go tool cover -func cover.out | grep total:
Expand Down
2 changes: 1 addition & 1 deletion cache/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (fs *ReadOnlyFS) Open(name string) (hackpadfs.File, error) {

func (fs *ReadOnlyFS) copyFile(name string, f hackpadfs.File, info hackpadfs.FileInfo) error {
parentName := path.Dir(name)
if err := hackpadfs.MkdirAll(fs.cacheFS, parentName, 0700); err != nil {
if err := hackpadfs.MkdirAll(fs.cacheFS, parentName, 0o700); err != nil {
return &hackpadfs.PathError{Op: "open", Path: parentName, Err: err}
}
destFile, err := fs.cacheFS.OpenFile(name, hackpadfs.FlagWriteOnly|hackpadfs.FlagCreate|hackpadfs.FlagTruncate, info.Mode())
Expand Down
2 changes: 1 addition & 1 deletion fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func Create(fs FS, name string) (File, error) {
if fs, ok := fs.(CreateFS); ok {
return fs.Create(name)
}
return OpenFile(fs, name, FlagReadWrite|FlagCreate|FlagTruncate, 0666)
return OpenFile(fs, name, FlagReadWrite|FlagCreate|FlagTruncate, 0o666)
}

// Mkdir creates a directory. Fails with a not implemented error if it's not a MkdirFS.
Expand Down
16 changes: 8 additions & 8 deletions fs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func TestMkdirAll(t *testing.T) {
t.Run("invalid path", func(t *testing.T) {
t.Parallel()
fs := makeSimplerFS(t)
err := hackpadfs.MkdirAll(fs, "foo/../bar", 0700)
err := hackpadfs.MkdirAll(fs, "foo/../bar", 0o700)
if assert.IsType(t, &hackpadfs.PathError{}, err) {
err := err.(*hackpadfs.PathError)
assert.Equal(t, "mkdirall", err.Op)
Expand All @@ -69,15 +69,15 @@ func TestMkdirAll(t *testing.T) {
t.Run("make all", func(t *testing.T) {
t.Parallel()
fs := makeSimplerFS(t)
err := hackpadfs.MkdirAll(fs, "foo/bar", 0700)
err := hackpadfs.MkdirAll(fs, "foo/bar", 0o700)
assert.NoError(t, err)
})

t.Run("make once", func(t *testing.T) {
t.Parallel()
fs := makeSimplerFS(t)
assert.NoError(t, fs.simpler.Mkdir("foo", 0600))
err := hackpadfs.MkdirAll(fs, "foo/bar", 0700)
assert.NoError(t, fs.simpler.Mkdir("foo", 0o600))
err := hackpadfs.MkdirAll(fs, "foo/bar", 0o700)
assert.NoError(t, err)
})

Expand All @@ -87,7 +87,7 @@ func TestMkdirAll(t *testing.T) {
f, err := hackpadfs.Create(fs.simpler, "foo")
requireNoError(t, err)
requireNoError(t, f.Close())
err = hackpadfs.MkdirAll(fs, "foo/bar", 0700)
err = hackpadfs.MkdirAll(fs, "foo/bar", 0o700)
if assert.IsType(t, &hackpadfs.PathError{}, err) {
err := err.(*hackpadfs.PathError)
assert.Equal(t, "mkdir", err.Op)
Expand All @@ -112,7 +112,7 @@ func TestWriteFullFile(t *testing.T) {
t.Parallel()

fs := makeSimplerFS(t)
err := hackpadfs.WriteFullFile(fs, "foo", []byte("bar"), 0756)
err := hackpadfs.WriteFullFile(fs, "foo", []byte("bar"), 0o756)
assert.NoError(t, err)
contents, err := hackpadfs.ReadFile(fs, "foo")
assert.NoError(t, err)
Expand All @@ -123,8 +123,8 @@ func TestRemoveAll(t *testing.T) {
t.Parallel()

fs := makeSimplerFS(t)
assert.NoError(t, hackpadfs.MkdirAll(fs, "foo/bar", 0700))
err := hackpadfs.WriteFullFile(fs, "foo/bar/baz", nil, 0700)
assert.NoError(t, hackpadfs.MkdirAll(fs, "foo/bar", 0o700))
err := hackpadfs.WriteFullFile(fs, "foo/bar/baz", nil, 0o700)
assert.NoError(t, err)

err = hackpadfs.RemoveAll(fs, "foo")
Expand Down
40 changes: 19 additions & 21 deletions fstest/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,10 @@ func TestFileRead(tb testing.TB, o FSOptions) {
})
}

//nolint:govet
func TestFileReadAt(tb testing.TB, o FSOptions) {
const fileContents = "hello world"
setupFS, commit := o.Setup.FS(tb)
assert.NoError(tb, hackpadfs.WriteFullFile(setupFS, "foo", []byte(fileContents), 0666))
assert.NoError(tb, hackpadfs.WriteFullFile(setupFS, "foo", []byte(fileContents), 0o666))
fs := commit()

for _, tc := range []struct {
Expand Down Expand Up @@ -245,7 +244,7 @@ func TestFileSeek(tb testing.TB, o FSOptions) {

o.tbRun(tb, "seek current", func(tb testing.TB) {
setupFS, commit := o.Setup.FS(tb)
assert.NoError(tb, hackpadfs.WriteFullFile(setupFS, "foo", []byte(fileContents), 0666))
assert.NoError(tb, hackpadfs.WriteFullFile(setupFS, "foo", []byte(fileContents), 0o666))

fs := commit()
file, err := fs.Open("foo")
Expand Down Expand Up @@ -326,7 +325,7 @@ func testFileWrite(tb testing.TB, o FSOptions, writer func(hackpadfs.File, []byt
o.tbRun(tb, "write-truncate-write-read", func(tb testing.TB) {
const fileContents = "hello world"
setupFS, commit := o.Setup.FS(tb)
assert.NoError(tb, hackpadfs.WriteFullFile(setupFS, "foo", []byte(fileContents), 0666))
assert.NoError(tb, hackpadfs.WriteFullFile(setupFS, "foo", []byte(fileContents), 0o666))

fs := commit()
file, err := hackpadfs.OpenFile(fs, "foo", hackpadfs.FlagReadWrite|hackpadfs.FlagTruncate, 0)
Expand Down Expand Up @@ -469,7 +468,7 @@ func TestFileReadDir(tb testing.TB, o FSOptions) {
if assert.NoError(tb, err) {
assert.NoError(tb, file.Close())
}
assert.NoError(tb, setupFS.Mkdir("bar", 0700))
assert.NoError(tb, setupFS.Mkdir("bar", 0o700))

fs := commit()
file, err = fs.Open(".")
Expand All @@ -482,8 +481,8 @@ func TestFileReadDir(tb testing.TB, o FSOptions) {
return entries[a].Name() < entries[b].Name()
})
o.assertSubsetQuickInfos(tb, []quickInfo{
{Name: "bar", Mode: hackpadfs.ModeDir | 0700, IsDir: true},
{Name: "foo", Mode: 0666},
{Name: "bar", Mode: hackpadfs.ModeDir | 0o700, IsDir: true},
{Name: "foo", Mode: 0o666},
}, asQuickDirInfos(tb, entries))
})

Expand All @@ -493,7 +492,7 @@ func TestFileReadDir(tb testing.TB, o FSOptions) {
if assert.NoError(tb, err) {
assert.NoError(tb, file.Close())
}
assert.NoError(tb, setupFS.Mkdir("bar", 0700))
assert.NoError(tb, setupFS.Mkdir("bar", 0o700))

fs := commit()
file, err = fs.Open(".")
Expand All @@ -519,14 +518,14 @@ func TestFileReadDir(tb testing.TB, o FSOptions) {
assert.Equal(tb, 2, len(entries))
o.assertSubsetQuickInfos(tb, asQuickDirInfos(tb, entries), asQuickDirInfos(tb, entriesAll))
o.assertSubsetQuickInfos(tb, []quickInfo{
{Name: "bar", Mode: hackpadfs.ModeDir | 0700, IsDir: true},
{Name: "foo", Mode: 0666},
{Name: "bar", Mode: hackpadfs.ModeDir | 0o700, IsDir: true},
{Name: "foo", Mode: 0o666},
}, asQuickDirInfos(tb, entriesAll))
})

o.tbRun(tb, "readdir high N", func(tb testing.TB) {
setupFS, commit := o.Setup.FS(tb)
assert.NoError(tb, setupFS.Mkdir("bar", 0700))
assert.NoError(tb, setupFS.Mkdir("bar", 0o700))

fs := commit()
file, err := fs.Open(".")
Expand All @@ -536,14 +535,14 @@ func TestFileReadDir(tb testing.TB, o FSOptions) {
assert.NoError(tb, err)
assert.NoError(tb, file.Close())
o.assertSubsetQuickInfos(tb,
[]quickInfo{{Name: "bar", Mode: hackpadfs.ModeDir | 0700, IsDir: true}},
[]quickInfo{{Name: "bar", Mode: hackpadfs.ModeDir | 0o700, IsDir: true}},
asQuickDirInfos(tb, entries),
)
})

o.tbRun(tb, "list empty subdirectory", func(tb testing.TB) {
setupFS, commit := o.Setup.FS(tb)
assert.NoError(tb, setupFS.Mkdir("foo", 0700))
assert.NoError(tb, setupFS.Mkdir("foo", 0o700))

fs := commit()
file, err := fs.Open("foo")
Expand All @@ -557,7 +556,7 @@ func TestFileReadDir(tb testing.TB, o FSOptions) {

o.tbRun(tb, "list subdirectory", func(tb testing.TB) {
setupFS, commit := o.Setup.FS(tb)
assert.NoError(tb, setupFS.Mkdir("foo", 0700))
assert.NoError(tb, setupFS.Mkdir("foo", 0o700))
file, err := hackpadfs.Create(setupFS, "foo/bar")
if assert.NoError(tb, err) {
assert.NoError(tb, file.Close())
Expand All @@ -566,7 +565,7 @@ func TestFileReadDir(tb testing.TB, o FSOptions) {
if assert.NoError(tb, err) {
assert.NoError(tb, file.Close())
}
assert.NoError(tb, setupFS.Mkdir("foo/boo", 0700))
assert.NoError(tb, setupFS.Mkdir("foo/boo", 0o700))

fs := commit()
file, err = fs.Open("foo")
Expand All @@ -579,9 +578,9 @@ func TestFileReadDir(tb testing.TB, o FSOptions) {
return entries[a].Name() < entries[b].Name()
})
o.assertEqualQuickInfos(tb, []quickInfo{
{Name: "bar", Mode: 0666},
{Name: "baz", Mode: 0666},
{Name: "boo", Mode: hackpadfs.ModeDir | 0700, IsDir: true},
{Name: "bar", Mode: 0o666},
{Name: "baz", Mode: 0o666},
{Name: "boo", Mode: hackpadfs.ModeDir | 0o700, IsDir: true},
}, asQuickDirInfos(tb, entries))
})

Expand Down Expand Up @@ -654,7 +653,6 @@ func TestFileSync(tb testing.TB, o FSOptions) {
assert.NoError(tb, file.Close())
}

//nolint:govet
func TestFileTruncate(tb testing.TB, o FSOptions) {
const fileContents = "hello world"
for _, tc := range []struct {
Expand Down Expand Up @@ -704,12 +702,12 @@ func TestFileTruncate(tb testing.TB, o FSOptions) {
Err: tc.expectErrKind,
}, err)
o.tryAssertEqualFS(tb, map[string]fsEntry{
"foo": {Mode: 0666, Size: int64(len(fileContents))},
"foo": {Mode: 0o666, Size: int64(len(fileContents))},
}, fs)
} else {
assert.NoError(tb, err)
o.tryAssertEqualFS(tb, map[string]fsEntry{
"foo": {Mode: 0666, Size: tc.size},
"foo": {Mode: 0o666, Size: tc.size},
}, fs)
}
})
Expand Down
6 changes: 3 additions & 3 deletions fstest/file_concurrent.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
func TestConcurrentFileRead(tb testing.TB, o FSOptions) {
o.tbRun(tb, "same file path", func(tb testing.TB) {
setupFS, commit := o.Setup.FS(tb)
assert.NoError(tb, hackpadfs.WriteFullFile(setupFS, "foo", []byte("hello world"), 0666))
assert.NoError(tb, hackpadfs.WriteFullFile(setupFS, "foo", []byte("hello world"), 0o666))
fs := commit()
concurrentTasks(0, func(_ int) {
f, err := fs.Open("foo")
Expand All @@ -30,7 +30,7 @@ func TestConcurrentFileRead(tb testing.TB, o FSOptions) {
setupFS, commit := o.Setup.FS(tb)
const fileCount = 10
for i := 0; i < fileCount; i++ {
assert.NoError(tb, hackpadfs.WriteFullFile(setupFS, fmt.Sprintf("foo-%d", i), []byte("hello world"), 0666))
assert.NoError(tb, hackpadfs.WriteFullFile(setupFS, fmt.Sprintf("foo-%d", i), []byte("hello world"), 0o666))
}
fs := commit()
concurrentTasks(fileCount, func(i int) {
Expand Down Expand Up @@ -102,7 +102,7 @@ func TestConcurrentFileStat(tb testing.TB, o FSOptions) {
assert.NoError(tb, err)
assert.Equal(tb, quickInfo{
Name: "foo",
Mode: 0666,
Mode: 0o666,
}, asQuickInfo(info))
assert.NoError(tb, f.Close())
}
Expand Down
Loading

0 comments on commit b05f4b7

Please sign in to comment.