Fix race in filesystem_test.go

server/filesystem/disk_space.go

@@ -209,7 +209,8 @@ func (fs *Filesystem) hasSpaceFor(size int64) error {
 
 // Updates the disk usage for the Filesystem instance.
 func (fs *Filesystem) addDisk(i int64) int64 {
-	var size = atomic.LoadInt64(&fs.diskUsed)
+	size := atomic.LoadInt64(&fs.diskUsed)
+
 	// Sorry go gods. This is ugly but the best approach I can come up with for right
 	// now without completely re-evaluating the logic we use for determining disk space.
 	//

server/filesystem/filesystem_test.go

@@ -9,6 +9,7 @@ import (
 	"math/rand"
 	"os"
 	"path/filepath"
+	"sync/atomic"
 	"testing"
 	"unicode/utf8"
 )
@@ -329,7 +330,7 @@ func TestFilesystem_Readfile(t *testing.T) {
 
 		g.AfterEach(func() {
 			buf.Truncate(0)
-			fs.diskUsed = 0
+			atomic.StoreInt64(&fs.diskUsed, 0)
 			rfs.reset()
 		})
 	})
@@ -347,7 +348,7 @@ func TestFilesystem_Writefile(t *testing.T) {
 		g.It("can create a new file", func() {
 			r := bytes.NewReader([]byte("test file content"))
 
-			g.Assert(fs.diskUsed).Equal(int64(0))
+			g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(int64(0))
 
 			err := fs.Writefile("test.txt", r)
 			g.Assert(err).IsNil()
@@ -355,7 +356,7 @@ func TestFilesystem_Writefile(t *testing.T) {
 			err = fs.Readfile("test.txt", buf)
 			g.Assert(err).IsNil()
 			g.Assert(buf.String()).Equal("test file content")
-			g.Assert(fs.diskUsed).Equal(r.Size())
+			g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(r.Size())
 		})
 
 		g.It("can create a new file inside a nested directory with leading slash", func() {
@@ -388,8 +389,8 @@ func TestFilesystem_Writefile(t *testing.T) {
 			g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
 		})
 
-		g.It("cannot write a file that exceedes the disk limits", func() {
+		g.It("cannot write a file that exceeds the disk limits", func() {
-			fs.diskLimit = 1024
+			atomic.StoreInt64(&fs.diskLimit, 1024)
 
 			b := make([]byte, 1025)
 			_, err := rand.Read(b)
@@ -403,7 +404,7 @@ func TestFilesystem_Writefile(t *testing.T) {
 		})
 
 		g.It("updates the total space used when a file is appended to", func() {
-			fs.diskUsed = 100
+			atomic.StoreInt64(&fs.diskUsed, 100)
 
 			b := make([]byte, 100)
 			_, _ = rand.Read(b)
@@ -411,7 +412,7 @@ func TestFilesystem_Writefile(t *testing.T) {
 			r := bytes.NewReader(b)
 			err := fs.Writefile("test.txt", r)
 			g.Assert(err).IsNil()
-			g.Assert(fs.diskUsed).Equal(int64(200))
+			g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(int64(200))
 
 			// If we write less data than already exists, we should expect the total
 			// disk used to be decremented.
@@ -421,7 +422,7 @@ func TestFilesystem_Writefile(t *testing.T) {
 			r = bytes.NewReader(b)
 			err = fs.Writefile("test.txt", r)
 			g.Assert(err).IsNil()
-			g.Assert(fs.diskUsed).Equal(int64(150))
+			g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(int64(150))
 		})
 
 		g.It("truncates the file when writing new contents", func() {
@@ -441,8 +442,9 @@ func TestFilesystem_Writefile(t *testing.T) {
 		g.AfterEach(func() {
 			buf.Truncate(0)
 			rfs.reset()
-			fs.diskUsed = 0
+
-			fs.diskLimit = 0
+			atomic.StoreInt64(&fs.diskUsed, 0)
+			atomic.StoreInt64(&fs.diskLimit, 0)
 		})
 	})
 }
@@ -481,7 +483,7 @@ func TestFilesystem_CreateDirectory(t *testing.T) {
 		g.It("should not increment the disk usage", func() {
 			err := fs.CreateDirectory("test", "/")
 			g.Assert(err).IsNil()
-			g.Assert(fs.diskUsed).Equal(int64(0))
+			g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(int64(0))
 		})
 
 		g.AfterEach(func() {
@@ -597,7 +599,7 @@ func TestFilesystem_Copy(t *testing.T) {
 				panic(err)
 			}
 
-			fs.diskUsed = int64(utf8.RuneCountInString("test content"))
+			atomic.StoreInt64(&fs.diskUsed, int64(utf8.RuneCountInString("test content")))
 		})
 
 		g.It("should return an error if the source does not exist", func() {
@@ -640,7 +642,7 @@ func TestFilesystem_Copy(t *testing.T) {
 		})
 
 		g.It("should return an error if there is not space to copy the file", func() {
-			fs.diskLimit = 2
+			atomic.StoreInt64(&fs.diskLimit, 2)
 
 			err := fs.Copy("source.txt")
 			g.Assert(err).IsNotNil()
@@ -672,7 +674,7 @@ func TestFilesystem_Copy(t *testing.T) {
 				g.Assert(err).IsNil()
 			}
 
-			g.Assert(fs.diskUsed).Equal(int64(utf8.RuneCountInString("test content")) * 3)
+			g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(int64(utf8.RuneCountInString("test content")) * 3)
 		})
 
 		g.It("should create a copy inside of a directory", func() {
@@ -694,8 +696,9 @@ func TestFilesystem_Copy(t *testing.T) {
 
 		g.AfterEach(func() {
 			rfs.reset()
-			fs.diskUsed = 0
+
-			fs.diskLimit = 0
+			atomic.StoreInt64(&fs.diskUsed, 0)
+			atomic.StoreInt64(&fs.diskLimit, 0)
 		})
 	})
 }
@@ -710,7 +713,7 @@ func TestFilesystem_Delete(t *testing.T) {
 				panic(err)
 			}
 
-			fs.diskUsed = int64(utf8.RuneCountInString("test content"))
+			atomic.StoreInt64(&fs.diskUsed, int64(utf8.RuneCountInString("test content")))
 		})
 
 		g.It("does not delete files outside the root directory", func() {
@@ -744,7 +747,7 @@ func TestFilesystem_Delete(t *testing.T) {
 			g.Assert(err).IsNotNil()
 			g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue()
 
-			g.Assert(fs.diskUsed).Equal(int64(0))
+			g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(int64(0))
 		})
 
 		g.It("deletes all items inside a directory if the directory is deleted", func() {
@@ -762,11 +765,11 @@ func TestFilesystem_Delete(t *testing.T) {
 				g.Assert(err).IsNil()
 			}
 
-			fs.diskUsed = int64(utf8.RuneCountInString("test content") * 3)
+			atomic.StoreInt64(&fs.diskUsed, int64(utf8.RuneCountInString("test content")*3))
 
 			err = fs.Delete("foo")
 			g.Assert(err).IsNil()
-			g.Assert(fs.diskUsed).Equal(int64(0))
+			g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(int64(0))
 
 			for _, s := range sources {
 				_, err = rfs.StatServerFile(s)
@@ -777,8 +780,9 @@ func TestFilesystem_Delete(t *testing.T) {
 
 		g.AfterEach(func() {
 			rfs.reset()
-			fs.diskUsed = 0
+
-			fs.diskLimit = 0
+			atomic.StoreInt64(&fs.diskUsed, 0)
+			atomic.StoreInt64(&fs.diskLimit, 0)
 		})
 	})
 }