Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix data race when running go-git tests #28

Merged
merged 4 commits into from Dec 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions .github/workflows/test.yml
@@ -1,18 +1,19 @@
on: [push, pull_request]
name: Test
permissions: {}
jobs:
test:
strategy:
matrix:
go-version: [1.14.x, 1.15.x, 1.16.x]
go-version: [1.18.x,1.19.x]
platform: [ubuntu-latest, macos-latest, windows-latest]
runs-on: ${{ matrix.platform }}
steps:
- name: Install Go
uses: actions/setup-go@v1
uses: actions/setup-go@v3
with:
go-version: ${{ matrix.go-version }}
- name: Checkout code
uses: actions/checkout@v2
uses: actions/checkout@v3
- name: Test
run: go test ./...
run: make test
11 changes: 6 additions & 5 deletions .github/workflows/test_js.yml
@@ -1,24 +1,25 @@
on: [push, pull_request]
name: Test
name: Test JS
permissions: {}
jobs:
test:
strategy:
matrix:
go-version: [1.14.x, 1.15.x, 1.16.x]
go-version: [1.18.x,1.19.x]
runs-on: ubuntu-latest
steps:
- name: Install Go
uses: actions/setup-go@v1
uses: actions/setup-go@v3
with:
go-version: ${{ matrix.go-version }}

- name: Install wasmbrowsertest
run: |
go get github.com/agnivade/wasmbrowsertest
go install github.com/agnivade/wasmbrowsertest@latest
mv $HOME/go/bin/wasmbrowsertest $HOME/go/bin/go_js_wasm_exec
- name: Checkout code
uses: actions/checkout@v2
uses: actions/checkout@v3

- name: Test
run: go test -exec="$HOME/go/bin/go_js_wasm_exec" ./...
Expand Down
11 changes: 11 additions & 0 deletions Makefile
@@ -0,0 +1,11 @@
# Go parameters
GOCMD = go
GOTEST = $(GOCMD) test

.PHONY: test
test:
$(GOTEST) -race ./...

test-coverage:
echo "" > $(COVERAGE_REPORT); \
$(GOTEST) -coverprofile=$(COVERAGE_REPORT) -coverpkg=./... -covermode=$(COVERAGE_MODE) ./...
4 changes: 2 additions & 2 deletions go.mod
Expand Up @@ -3,8 +3,8 @@ module github.com/go-git/go-billy/v5
require (
github.com/kr/text v0.2.0 // indirect
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f
golang.org/x/sys v0.3.0
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c
)

go 1.13
8 changes: 8 additions & 0 deletions go.sum
@@ -1,6 +1,8 @@
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/go-git/go-billy v1.0.0 h1:bXR6Zu3opPSg0R4dDxqaLglY4rxw7ja7wS16qSpOKL4=
github.com/go-git/go-billy v3.1.0+incompatible h1:dwrJ8G2Jt1srYgIJs+lRjA36qBY68O2Lg5idKG8ef5M=
github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI=
github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
Expand All @@ -10,5 +12,11 @@ github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWb
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527 h1:uYVVQ9WP/Ds2ROhcaGPeIdVq0RIXVLwsHlnvJ+cT1So=
golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.2.0 h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A=
golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.3.0 h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ=
golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f h1:BLraFXnmrev5lT+xlilqcH8XK9/i0At2xKjWk4p6zsU=
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q=
11 changes: 10 additions & 1 deletion memfs/storage.go
Expand Up @@ -6,6 +6,7 @@ import (
"io"
"os"
"path/filepath"
"sync"
)

type storage struct {
Expand Down Expand Up @@ -174,6 +175,8 @@ func clean(path string) string {
type content struct {
name string
bytes []byte

m sync.RWMutex
}

func (c *content) WriteAt(p []byte, off int64) (int, error) {
Expand All @@ -185,6 +188,7 @@ func (c *content) WriteAt(p []byte, off int64) (int, error) {
}
}

c.m.Lock()
prev := len(c.bytes)

diff := int(off) - prev
Expand All @@ -196,6 +200,7 @@ func (c *content) WriteAt(p []byte, off int64) (int, error) {
if len(c.bytes) < prev {
c.bytes = c.bytes[:prev]
}
c.m.Unlock()

return len(p), nil
}
Expand All @@ -209,8 +214,10 @@ func (c *content) ReadAt(b []byte, off int64) (n int, err error) {
}
}

c.m.RLock()
size := int64(len(c.bytes))
if off >= size {
c.m.RUnlock()
return 0, io.EOF
}

Expand All @@ -220,10 +227,12 @@ func (c *content) ReadAt(b []byte, off int64) (n int, err error) {
}

btr := c.bytes[off : off+l]
n = copy(b, btr)

if len(btr) < len(b) {
err = io.EOF
}
n = copy(b, btr)
c.m.RUnlock()

return
}
65 changes: 33 additions & 32 deletions util/walk_test.go
Expand Up @@ -20,6 +20,7 @@ type WalkSuite struct{}
func TestWalk(t *testing.T) { TestingT(t) }

var _ = Suite(&WalkSuite{})
var targetSubfolder = filepath.FromSlash("path/to/some/subfolder")

func (s *WalkSuite) TestWalkCanSkipTopDirectory(c *C) {
filesystem := memfs.New()
Expand Down Expand Up @@ -52,13 +53,13 @@ func (s *WalkSuite) TestWalkOnExistingFolder(c *C) {
return nil
}), IsNil)
c.Assert(discoveredPaths, Contains, "path")
c.Assert(discoveredPaths, Contains, "path/to")
c.Assert(discoveredPaths, Contains, "path/to/some")
c.Assert(discoveredPaths, Contains, "path/to/some/file")
c.Assert(discoveredPaths, Contains, "path/to/some/subfolder")
c.Assert(discoveredPaths, Contains, "path/to/some/subfolder/that")
c.Assert(discoveredPaths, Contains, "path/to/some/subfolder/that/contain")
c.Assert(discoveredPaths, Contains, "path/to/some/subfolder/that/contain/file")
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to"))
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some"))
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/file"))
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/subfolder"))
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/subfolder/that"))
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/subfolder/that/contain"))
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/subfolder/that/contain/file"))
}

func (s *WalkSuite) TestWalkCanSkipFolder(c *C) {
Expand All @@ -68,19 +69,19 @@ func (s *WalkSuite) TestWalkCanSkipFolder(c *C) {
discoveredPaths := []string{}
c.Assert(util.Walk(filesystem, "path", func(path string, info os.FileInfo, err error) error {
discoveredPaths = append(discoveredPaths, path)
if path == "path/to/some/subfolder" {
if path == targetSubfolder {
return filepath.SkipDir
}
return nil
}), IsNil)
c.Assert(discoveredPaths, Contains, "path")
c.Assert(discoveredPaths, Contains, "path/to")
c.Assert(discoveredPaths, Contains, "path/to/some")
c.Assert(discoveredPaths, Contains, "path/to/some/file")
c.Assert(discoveredPaths, Contains, "path/to/some/subfolder")
c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that")
c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that/contain")
c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that/contain/file")
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to"))
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some"))
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/file"))
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/subfolder"))
c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that"))
c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that/contain"))
c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that/contain/file"))
}

func (s *WalkSuite) TestWalkStopsOnError(c *C) {
Expand All @@ -90,27 +91,27 @@ func (s *WalkSuite) TestWalkStopsOnError(c *C) {
discoveredPaths := []string{}
c.Assert(util.Walk(filesystem, "path", func(path string, info os.FileInfo, err error) error {
discoveredPaths = append(discoveredPaths, path)
if path == "path/to/some/subfolder" {
if path == targetSubfolder {
return errors.New("uncaught error")
}
return nil
}), NotNil)
c.Assert(discoveredPaths, Contains, "path")
c.Assert(discoveredPaths, Contains, "path/to")
c.Assert(discoveredPaths, Contains, "path/to/some")
c.Assert(discoveredPaths, Contains, "path/to/some/file")
c.Assert(discoveredPaths, Contains, "path/to/some/subfolder")
c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that")
c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that/contain")
c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that/contain/file")
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to"))
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some"))
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/file"))
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/subfolder"))
c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that"))
c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that/contain"))
c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that/contain/file"))
}

func (s *WalkSuite) TestWalkForwardsStatErrors(c *C) {
memFilesystem := memfs.New()
filesystem := &fnFs{
Filesystem: memFilesystem,
lstat: func(path string) (os.FileInfo, error) {
if path == "path/to/some/subfolder" {
if path == targetSubfolder {
return nil, errors.New("uncaught error")
}
return memFilesystem.Lstat(path)
Expand All @@ -122,19 +123,19 @@ func (s *WalkSuite) TestWalkForwardsStatErrors(c *C) {
discoveredPaths := []string{}
c.Assert(util.Walk(filesystem, "path", func(path string, info os.FileInfo, err error) error {
discoveredPaths = append(discoveredPaths, path)
if path == "path/to/some/subfolder" {
if path == targetSubfolder {
c.Assert(err, NotNil)
}
return err
}), NotNil)
c.Assert(discoveredPaths, Contains, "path")
c.Assert(discoveredPaths, Contains, "path/to")
c.Assert(discoveredPaths, Contains, "path/to/some")
c.Assert(discoveredPaths, Contains, "path/to/some/file")
c.Assert(discoveredPaths, Contains, "path/to/some/subfolder")
c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that")
c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that/contain")
c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that/contain/file")
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to"))
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some"))
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/file"))
c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/subfolder"))
c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that"))
c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that/contain"))
c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that/contain/file"))
}

func createFile(c *C, filesystem billy.Filesystem, path string) {
Expand Down