Skip to content

Commit eeabec1

Browse files
Add some tests and cleanup
1 parent 67a31fe commit eeabec1

File tree

6 files changed

+210
-233
lines changed

6 files changed

+210
-233
lines changed

pkg/filesystem/pool/quota_enforcing_file_pool.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ type quotaEnforcingFile struct {
4545
filesystem.FileReadWriter
4646

4747
pool *quotaEnforcingFilePool
48-
size int64
4948
}
5049

5150
func (f *quotaEnforcingFile) Close() error {
Lines changed: 65 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -1,140 +1,67 @@
11
package pool_test
22

3-
// import (
4-
// "io"
5-
// "testing"
6-
7-
// "github.com/buildbarn/bb-remote-execution/internal/mock"
8-
// "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool"
9-
// "github.com/buildbarn/bb-storage/pkg/filesystem"
10-
// "github.com/stretchr/testify/require"
11-
12-
// "google.golang.org/grpc/codes"
13-
// "google.golang.org/grpc/status"
14-
15-
// "go.uber.org/mock/gomock"
16-
// )
17-
18-
// // testRemainingQuota is a helper function for the
19-
// // QuotaEnforcingFilePool tests to check that a certain amount of space
20-
// // is available within the pool.
21-
// func testRemainingQuota(t *testing.T, ctrl *gomock.Controller, underlyingPool *mock.MockFilePool, pool pool.FilePool, filesRemaining int, bytesRemaining int64) {
22-
// // Check that the remaining number of files is available by
23-
// // allocating all of them.
24-
// underlyingFiles := make([]*mock.MockFileReadWriter, filesRemaining)
25-
// files := make([]filesystem.FileReadWriter, filesRemaining)
26-
// for i := 0; i < filesRemaining; i++ {
27-
// underlyingFiles[i] = mock.NewMockFileReadWriter(ctrl)
28-
// underlyingPool.EXPECT().NewFile().Return(underlyingFiles[i], nil)
29-
// var err error
30-
// files[i], err = pool.NewFile()
31-
// require.NoError(t, err)
32-
// }
33-
// _, err := pool.NewFile()
34-
// require.Equal(t, err, status.Error(codes.InvalidArgument, "File count quota reached"))
35-
// for i := 0; i < filesRemaining; i++ {
36-
// underlyingFiles[i].EXPECT().Close().Return(nil)
37-
// require.NoError(t, files[i].Close())
38-
// }
39-
40-
// // Check that the remaining amount of space is available by
41-
// // allocating one file and truncating it to the exact size.
42-
// underlyingFile := mock.NewMockFileReadWriter(ctrl)
43-
// underlyingPool.EXPECT().NewFile().Return(underlyingFile, nil)
44-
// f, err := pool.NewFile()
45-
// require.NoError(t, err)
46-
// if bytesRemaining != 0 {
47-
// underlyingFile.EXPECT().Truncate(bytesRemaining).Return(nil)
48-
// }
49-
// require.NoError(t, f.Truncate(bytesRemaining))
50-
// require.Equal(t, f.Truncate(bytesRemaining+1), status.Error(codes.InvalidArgument, "File size quota reached"))
51-
// underlyingFile.EXPECT().Close().Return(nil)
52-
// require.NoError(t, f.Close())
53-
// }
54-
55-
// func TestQuotaEnforcingFilePoolExample(t *testing.T) {
56-
// ctrl := gomock.NewController(t)
57-
58-
// // An empty pool should have the advertised amount of space available.
59-
// underlyingPool := mock.NewMockFilePool(ctrl)
60-
// pool := pool.NewQuotaEnforcingFilePool(underlyingPool, 10, 1000)
61-
// testRemainingQuota(t, ctrl, underlyingPool, pool, 10, 1000)
62-
63-
// // Failure to allocate a file from the underlying pool should
64-
// // not affect the quota.
65-
// underlyingPool.EXPECT().NewFile().Return(nil, status.Error(codes.Internal, "I/O error"))
66-
// _, err := pool.NewFile()
67-
// require.Equal(t, err, status.Error(codes.Internal, "I/O error"))
68-
// testRemainingQuota(t, ctrl, underlyingPool, pool, 10, 1000)
69-
70-
// // Successfully allocate a file.
71-
// underlyingFile := mock.NewMockFileReadWriter(ctrl)
72-
// underlyingPool.EXPECT().NewFile().Return(underlyingFile, nil)
73-
// f, err := pool.NewFile()
74-
// require.NoError(t, err)
75-
// testRemainingQuota(t, ctrl, underlyingPool, pool, 9, 1000)
76-
77-
// // Read calls should be forwarded properly.
78-
// var p [10]byte
79-
// underlyingFile.EXPECT().ReadAt(p[:], int64(123)).Return(0, io.EOF)
80-
// n, err := f.ReadAt(p[:], 123)
81-
// require.Equal(t, 0, n)
82-
// require.Equal(t, io.EOF, err)
83-
// testRemainingQuota(t, ctrl, underlyingPool, pool, 9, 1000)
84-
85-
// // Writes that would cause the file to grow beyond the maximum
86-
// // size should be disallowed.
87-
// n, err = f.WriteAt(p[:], 991)
88-
// require.Equal(t, 0, n)
89-
// require.Equal(t, err, status.Error(codes.InvalidArgument, "File size quota reached"))
90-
// testRemainingQuota(t, ctrl, underlyingPool, pool, 9, 1000)
91-
92-
// // A failed write should initially allocate all of the required
93-
// // space, but release the full amount once more.
94-
// underlyingFile.EXPECT().WriteAt(p[:], int64(990)).Return(0, status.Error(codes.Internal, "Cannot write data at all"))
95-
// n, err = f.WriteAt(p[:], 990)
96-
// require.Equal(t, 0, n)
97-
// require.Equal(t, err, status.Error(codes.Internal, "Cannot write data at all"))
98-
// testRemainingQuota(t, ctrl, underlyingPool, pool, 9, 1000)
99-
100-
// // A short write should initially allocate all of the required
101-
// // space, but release the amount of data that was not written.
102-
// underlyingFile.EXPECT().WriteAt(p[:], int64(990)).Return(7, status.Error(codes.Internal, "Disk died in the middle of the write"))
103-
// n, err = f.WriteAt(p[:], 990)
104-
// require.Equal(t, 7, n)
105-
// require.Equal(t, err, status.Error(codes.Internal, "Disk died in the middle of the write"))
106-
// testRemainingQuota(t, ctrl, underlyingPool, pool, 9, 3)
107-
108-
// // I/O error while shrinking file should not cause the quotas to
109-
// // be affected.
110-
// underlyingFile.EXPECT().Truncate(int64(123)).Return(status.Error(codes.Internal, "Failed to adjust inode"))
111-
// require.Equal(t, f.Truncate(123), status.Error(codes.Internal, "Failed to adjust inode"))
112-
// testRemainingQuota(t, ctrl, underlyingPool, pool, 9, 3)
113-
114-
// // Successfully shrinking the file.
115-
// underlyingFile.EXPECT().Truncate(int64(123)).Return(nil)
116-
// require.NoError(t, f.Truncate(123))
117-
// testRemainingQuota(t, ctrl, underlyingPool, pool, 9, 877)
118-
119-
// // Growing the file past the permitted size should not be
120-
// // allowed.
121-
// require.Equal(t, f.Truncate(1001), status.Error(codes.InvalidArgument, "File size quota reached"))
122-
// testRemainingQuota(t, ctrl, underlyingPool, pool, 9, 877)
123-
124-
// // I/O error while growing file should not cause the quotas to
125-
// // be affected.
126-
// underlyingFile.EXPECT().Truncate(int64(1000)).Return(status.Error(codes.Internal, "Failed to adjust inode"))
127-
// require.Equal(t, f.Truncate(1000), status.Error(codes.Internal, "Failed to adjust inode"))
128-
// testRemainingQuota(t, ctrl, underlyingPool, pool, 9, 877)
129-
130-
// // Successfully growing the file.
131-
// underlyingFile.EXPECT().Truncate(int64(1000)).Return(nil)
132-
// require.NoError(t, f.Truncate(1000))
133-
// testRemainingQuota(t, ctrl, underlyingPool, pool, 9, 0)
134-
135-
// // Closing the file should bring the pool back in the initial
136-
// // state.
137-
// underlyingFile.EXPECT().Close().Return(nil)
138-
// require.NoError(t, f.Close())
139-
// testRemainingQuota(t, ctrl, underlyingPool, pool, 10, 1000)
140-
// }
3+
import (
4+
"testing"
5+
6+
"github.com/buildbarn/bb-remote-execution/internal/mock"
7+
"github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool"
8+
"github.com/buildbarn/bb-storage/pkg/filesystem"
9+
"github.com/stretchr/testify/require"
10+
11+
"google.golang.org/grpc/codes"
12+
"google.golang.org/grpc/status"
13+
14+
"go.uber.org/mock/gomock"
15+
)
16+
17+
// testRemainingFileQuota is a helper function for the
18+
// QuotaEnforcingFilePool tests to check that a certain amount of space
19+
// is available within the pool.
20+
func testRemainingFileQuota(t *testing.T, ctrl *gomock.Controller, underlyingPool *mock.MockFilePool, pool pool.FilePool, filesRemaining int) {
21+
// Check that the remaining number of files is available by
22+
// allocating all of them.
23+
underlyingFiles := make([]*mock.MockFileReadWriter, filesRemaining)
24+
files := make([]filesystem.FileReadWriter, filesRemaining)
25+
for i := 0; i < filesRemaining; i++ {
26+
underlyingFiles[i] = mock.NewMockFileReadWriter(ctrl)
27+
underlyingPool.EXPECT().NewFile().Return(underlyingFiles[i], nil)
28+
var err error
29+
files[i], err = pool.NewFile()
30+
require.NoError(t, err)
31+
}
32+
_, err := pool.NewFile()
33+
require.Equal(t, err, status.Error(codes.InvalidArgument, "File count quota reached"))
34+
for i := 0; i < filesRemaining; i++ {
35+
underlyingFiles[i].EXPECT().Close().Return(nil)
36+
require.NoError(t, files[i].Close())
37+
}
38+
}
39+
40+
func TestQuotaEnforcingFilePoolExample(t *testing.T) {
41+
ctrl := gomock.NewController(t)
42+
43+
// An empty pool should have the advertised amount of space available.
44+
underlyingPool := mock.NewMockFilePool(ctrl)
45+
pool := pool.NewQuotaEnforcingFilePool(underlyingPool, 10)
46+
testRemainingFileQuota(t, ctrl, underlyingPool, pool, 10)
47+
48+
// Failure to allocate a file from the underlying pool should
49+
// not affect the quota.
50+
underlyingPool.EXPECT().NewFile().Return(nil, status.Error(codes.Internal, "I/O error"))
51+
_, err := pool.NewFile()
52+
require.Equal(t, err, status.Error(codes.Internal, "I/O error"))
53+
testRemainingFileQuota(t, ctrl, underlyingPool, pool, 10)
54+
55+
// Successfully allocate a file.
56+
underlyingFile := mock.NewMockFileReadWriter(ctrl)
57+
underlyingPool.EXPECT().NewFile().Return(underlyingFile, nil)
58+
f, err := pool.NewFile()
59+
require.NoError(t, err)
60+
testRemainingFileQuota(t, ctrl, underlyingPool, pool, 9)
61+
62+
// Closing the file should bring the pool back in the initial
63+
// state.
64+
underlyingFile.EXPECT().Close().Return(nil)
65+
require.NoError(t, f.Close())
66+
testRemainingFileQuota(t, ctrl, underlyingPool, pool, 10)
67+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,45 @@
11
package pool_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/buildbarn/bb-remote-execution/internal/mock"
7+
"github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool"
8+
"github.com/stretchr/testify/require"
9+
10+
"google.golang.org/grpc/codes"
11+
"google.golang.org/grpc/status"
12+
13+
"go.uber.org/mock/gomock"
14+
)
15+
16+
// testRemainingSectorQuota is a helper function for the
17+
// QuotaEnforcingSectorAllocator tests to check that a certain amount of
18+
// space is available within the allocator..
19+
func testRemainingSectorQuota(t *testing.T, ctrl *gomock.Controller, underlyingAllocator *mock.MockSectorAllocator, allocator pool.SectorAllocator, sectorsRemaining int64) {
20+
// Check that the remaining number of sectors are available by
21+
// allocating all of them.
22+
sectors := make([]uint32, sectorsRemaining)
23+
for i := 0; i < int(sectorsRemaining); i++ {
24+
underlyingAllocator.EXPECT().AllocateContiguous(gomock.Any()).Return(uint32(i), 1, nil)
25+
sector, count, err := allocator.AllocateContiguous(1)
26+
sectors[i] = sector
27+
require.NoError(t, err)
28+
require.Equal(t, count, 1)
29+
}
30+
_, _, err := allocator.AllocateContiguous(1)
31+
require.Equal(t, err, status.Error(codes.InvalidArgument, "Sector count quota reached"))
32+
underlyingAllocator.EXPECT().FreeList(gomock.Any())
33+
allocator.FreeList(sectors)
34+
}
35+
36+
func TestQuotaEnforcingSectorAllocator(t *testing.T) {
37+
ctrl := gomock.NewController(t)
38+
39+
underlyingAllocator := mock.NewMockSectorAllocator(ctrl)
40+
allocator := pool.NewQuotaEnforcingSectorAllocator(underlyingAllocator, 100)
41+
42+
// All of the sectors should be available.
43+
testRemainingSectorQuota(t, ctrl, underlyingAllocator, allocator, 100)
44+
45+
}

pkg/filesystem/pool/sector_mapper.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ type SectorMapper interface {
99
// lists exist then the function returns an empty list.
1010
GetNextDirectSectorList(start uint32) (uint32, []uint32)
1111

12-
// GetSectorFromIndex returns the physical sector index for a
12+
// GetPhysicalIndex returns the physical sector index for a
1313
// given logical index. This physical index may be zero which
1414
// indicates that this logical sector is unmapped.
1515
GetPhysicalIndex(index uint32) uint32

0 commit comments

Comments
 (0)