Skip to content

Commit ae2809c

Browse files
RaduBerindeRaduBerinde
committed
objstorageprovider: clean up datadriven tests
Change TestProvider to use key=value format for arguments, making the test easier to read and simplifying the test code.
1 parent a8211a0 commit ae2809c

File tree

11 files changed

+219
-221
lines changed

11 files changed

+219
-221
lines changed

‎objstorage/objstorageprovider/provider_test.go‎

Lines changed: 69 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"context"
99
"fmt"
1010
"math/rand/v2"
11-
"slices"
1211
"strings"
1312
"sync"
1413
"sync/atomic"
@@ -24,6 +23,16 @@ import (
2423
"github.com/stretchr/testify/require"
2524
)
2625

26+
// TestProvider datadriven format:
27+
//
28+
// open dir=<dir> [cold-dir=<dir>] [creator-id=<id>]
29+
// create file-num=<num> [file-type=<type>] [shared] [cold-tier] salt=<salt> size=<size> [no-ref-tracking]
30+
// read file-num=<num> [file-type=<type>] [for-compaction] [readahead=<mode>]
31+
// remove file-num=<num> [file-type=<type>]
32+
// link-or-copy file-num=<num> [file-type=<type>] [shared] salt=<salt> size=<size> [no-ref-tracking]
33+
// save-backing key=<key> file-num=<num>
34+
// close-backing key=<key>
35+
// switch dir=<dir>
2736
func TestProvider(t *testing.T) {
2837
datadriven.Walk(t, "testdata/provider", func(t *testing.T, path string) {
2938
var log base.InMemLogger
@@ -46,40 +55,40 @@ func TestProvider(t *testing.T) {
4655
var curProvider objstorage.Provider
4756
readaheadConfig := NewReadaheadConfig()
4857
datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string {
58+
// Arguments that are common to multiple commands.
59+
var fsDir, coldDir string
60+
var fileType base.FileType
61+
var fileNum base.DiskFileNum
62+
var salt, size int
63+
var noRefTracking, shared, coldTier bool
64+
65+
d.MaybeScanArgs(t, "dir", &fsDir)
66+
d.MaybeScanArgs(t, "cold-dir", &coldDir)
67+
fileType = func() base.FileType {
68+
var fileTypeStr string
69+
if !d.MaybeScanArgs(t, "file-type", &fileTypeStr) {
70+
return base.FileTypeTable
71+
}
72+
return base.FileTypeFromName(fileTypeStr)
73+
}()
74+
d.MaybeScanArgs(t, "file-num", &fileNum)
75+
d.MaybeScanArgs(t, "salt", &salt)
76+
d.MaybeScanArgs(t, "size", &size)
77+
noRefTracking = d.HasArg("no-ref-tracking")
78+
shared = d.HasArg("shared")
79+
coldTier = d.HasArg("cold-tier")
80+
4981
readaheadConfig.Set(defaultReadaheadInformed, defaultReadaheadSpeculative)
50-
scanArgs := func(desc string, args ...interface{}) {
51-
t.Helper()
52-
if len(d.CmdArgs) != len(args) {
53-
d.Fatalf(t, "usage: %s %s", d.Cmd, desc)
54-
}
55-
for i := range args {
56-
_, err := fmt.Sscan(d.CmdArgs[i].String(), args[i])
57-
if err != nil {
58-
d.Fatalf(t, "%s: error parsing argument '%s'", d.Cmd, d.CmdArgs[i])
59-
}
60-
}
61-
}
62-
ctx := context.Background()
6382

83+
ctx := context.Background()
6484
log.Reset()
6585
switch d.Cmd {
6686
case "open":
67-
var fsDir, coldDir string
87+
if fsDir == "" {
88+
d.Fatalf(t, "usage: switch dir=<dir> [cold-dir=<dir>] [creator-id=<id>]")
89+
}
6890
var creatorID objstorage.CreatorID
69-
d.CmdArgs = slices.DeleteFunc(d.CmdArgs, func(arg datadriven.CmdArg) bool {
70-
switch arg.Key {
71-
case "creator-id":
72-
var id uint64
73-
arg.Scan(t, 0, &id)
74-
creatorID = objstorage.CreatorID(id)
75-
return true
76-
case "cold-tier":
77-
coldDir = arg.SingleVal(t)
78-
return true
79-
}
80-
return false
81-
})
82-
scanArgs("<fs-dir> [creator-id=X]", &fsDir)
91+
d.MaybeScanArgs(t, "creator-id", &creatorID)
8392

8493
require.NoError(t, fs.MkdirAll(fsDir, 0755))
8594
st := DefaultSettings(fs, fsDir)
@@ -111,8 +120,9 @@ func TestProvider(t *testing.T) {
111120
return log.String()
112121

113122
case "switch":
114-
var fsDir string
115-
scanArgs("<fs-dir>", &fsDir)
123+
if fsDir == "" {
124+
d.Fatalf(t, "usage: switch dir=<dir>")
125+
}
116126
curProvider = providers[fsDir]
117127
if curProvider == nil {
118128
t.Fatalf("unknown provider %s", fsDir)
@@ -129,36 +139,17 @@ func TestProvider(t *testing.T) {
129139
return log.String()
130140

131141
case "create":
142+
if fileNum == 0 || size == 0 || salt == 0 {
143+
d.Fatalf(t, "usage: create file-num=<num> [file-type=sstable|blob] [shared] [cold-tier] salt=<salt> size=<size> [no-ref-tracking]")
144+
}
132145
opts := objstorage.CreateOptions{
133146
SharedCleanupMethod: objstorage.SharedRefTracking,
134147
}
135-
ft := base.FileTypeTable
136-
d.CmdArgs = slices.DeleteFunc(d.CmdArgs, func(arg datadriven.CmdArg) bool {
137-
switch arg.Key {
138-
case "file-type":
139-
ft = base.FileTypeFromName(d.CmdArgs[0].FirstVal(t))
140-
return true
141-
case "no-ref-tracking":
142-
opts.SharedCleanupMethod = objstorage.SharedNoCleanup
143-
return true
144-
case "cold-tier":
145-
opts.Tier = base.ColdTier
146-
return true
147-
}
148-
return false
149-
})
150-
var fileNum base.DiskFileNum
151-
var typ string
152-
var salt, size int
153-
scanArgs("[file-type=sstable|blob] <file-num> <local|shared> <salt> <size> [no-ref-tracking] [cold-tier]", &fileNum, &typ, &salt, &size)
154-
switch typ {
155-
case "local":
156-
case "shared":
157-
opts.PreferSharedStorage = true
158-
default:
159-
d.Fatalf(t, "'%s' should be 'local' or 'shared'", typ)
160-
}
161-
w, _, err := curProvider.Create(ctx, ft, fileNum, opts)
148+
opts.PreferSharedStorage = shared
149+
if coldTier {
150+
opts.Tier = base.ColdTier
151+
}
152+
w, _, err := curProvider.Create(ctx, fileType, fileNum, opts)
162153
if err != nil {
163154
return err.Error()
164155
}
@@ -171,29 +162,16 @@ func TestProvider(t *testing.T) {
171162
return log.String()
172163

173164
case "link-or-copy":
165+
if fileNum == 0 || size == 0 || salt == 0 {
166+
d.Fatalf(t, "usage: link-or-copy file-num=<num> [file-type=sstable|blob] [shared] [cold-tier] salt=<salt> size=<size> [no-ref-tracking]")
167+
}
174168
opts := objstorage.CreateOptions{
175169
SharedCleanupMethod: objstorage.SharedRefTracking,
176170
}
177-
ft := base.FileTypeTable
178-
if len(d.CmdArgs) > 0 && d.CmdArgs[0].Key == "file-type" {
179-
ft = base.FileTypeFromName(d.CmdArgs[0].FirstVal(t))
180-
d.CmdArgs = d.CmdArgs[1:]
181-
}
182-
if len(d.CmdArgs) == 5 && d.CmdArgs[4].Key == "no-ref-tracking" {
183-
d.CmdArgs = d.CmdArgs[:4]
171+
if noRefTracking {
184172
opts.SharedCleanupMethod = objstorage.SharedNoCleanup
185173
}
186-
var fileNum base.DiskFileNum
187-
var typ string
188-
var salt, size int
189-
scanArgs("[file-type=sstable|blob] <file-num> <local|shared> <salt> <size> [no-ref-tracking]", &fileNum, &typ, &salt, &size)
190-
switch typ {
191-
case "local":
192-
case "shared":
193-
opts.PreferSharedStorage = true
194-
default:
195-
d.Fatalf(t, "'%s' should be 'local' or 'shared'", typ)
196-
}
174+
opts.PreferSharedStorage = shared
197175

198176
tmpFileCounter++
199177
tmpFilename := fmt.Sprintf("temp-file-%d", tmpFileCounter)
@@ -206,11 +184,14 @@ func TestProvider(t *testing.T) {
206184
require.NoError(t, err)
207185
require.NoError(t, f.Close())
208186

209-
_, err = curProvider.LinkOrCopyFromLocal(ctx, fs, tmpFilename, ft, fileNum, opts)
187+
_, err = curProvider.LinkOrCopyFromLocal(ctx, fs, tmpFilename, fileType, fileNum, opts)
210188
require.NoError(t, err)
211189
return log.String()
212190

213191
case "read":
192+
if fileNum == 0 {
193+
d.Fatalf(t, "usage: read file-num=<num> [file-type=sstable|blob] <file-num> [for-compaction] [readahead|speculative-overhead=off|sys-readahead|fadvise-sequential]")
194+
}
214195
forCompaction := d.HasArg("for-compaction")
215196
if arg, ok := d.Arg("readahead"); ok {
216197
var mode ReadaheadMode
@@ -231,15 +212,7 @@ func TestProvider(t *testing.T) {
231212
}
232213
}
233214

234-
ft := base.FileTypeTable
235-
if len(d.CmdArgs) > 0 && d.CmdArgs[0].Key == "file-type" {
236-
ft = base.FileTypeFromName(d.CmdArgs[0].FirstVal(t))
237-
d.CmdArgs = d.CmdArgs[1:]
238-
}
239-
d.CmdArgs = d.CmdArgs[:1]
240-
var fileNum base.DiskFileNum
241-
scanArgs("[file-type=sstable|blob] <file-num> [for-compaction] [readahead|speculative-overhead=off|sys-readahead|fadvise-sequential]", &fileNum)
242-
r, err := curProvider.OpenForReading(ctx, ft, fileNum, objstorage.OpenOptions{})
215+
r, err := curProvider.OpenForReading(ctx, fileType, fileNum, objstorage.OpenOptions{})
243216
if err != nil {
244217
return err.Error()
245218
}
@@ -272,14 +245,10 @@ func TestProvider(t *testing.T) {
272245
return log.String()
273246

274247
case "remove":
275-
ft := base.FileTypeTable
276-
if len(d.CmdArgs) > 0 && d.CmdArgs[0].Key == "file-type" {
277-
ft = base.FileTypeFromName(d.CmdArgs[0].FirstVal(t))
278-
d.CmdArgs = d.CmdArgs[1:]
279-
}
280-
var fileNum base.DiskFileNum
281-
scanArgs("[file-type=sstable|blob] <file-num>", &fileNum)
282-
if err := curProvider.Remove(ft, fileNum); err != nil {
248+
if fileNum == 0 {
249+
d.Fatalf(t, "usage: remove file-num=<num> [file-type=sstable|blob]")
250+
}
251+
if err := curProvider.Remove(fileType, fileNum); err != nil {
283252
return err.Error()
284253
}
285254
return log.String()
@@ -291,9 +260,12 @@ func TestProvider(t *testing.T) {
291260
return log.String()
292261

293262
case "save-backing":
263+
if fileNum == 0 {
264+
d.Fatalf(t, "usage: save-backing file-num=<num> [file-type=sstable|blob] key=<key>")
265+
}
294266
var key string
295-
var fileNum base.DiskFileNum
296-
scanArgs("<key> <file-num>", &key, &fileNum)
267+
d.ScanArgs(t, "key", &key)
268+
297269
meta, err := curProvider.Lookup(base.FileTypeTable, fileNum)
298270
require.NoError(t, err)
299271
var handle objstorage.RemoteObjectBackingHandle
@@ -312,7 +284,7 @@ func TestProvider(t *testing.T) {
312284

313285
case "close-backing":
314286
var key string
315-
scanArgs("<key>", &key)
287+
d.ScanArgs(t, "key", &key)
316288
backingHandles[key].Close()
317289
return ""
318290

‎objstorage/objstorageprovider/testdata/provider/cold_tier‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
1-
open p1 cold-tier=cold1
1+
open dir=p1 cold-dir=cold1
22
----
33
<local fs> mkdir-all: p1 0755
44
<local fs> mkdir-all: cold1 0755
55
<local fs> open-dir: p1
66
<local fs> open-dir: cold1
77

88
# Create a cold file.
9-
create file-type=blob 1 local 1 1024 cold-tier
9+
create file-type=blob file-num=1 cold-tier salt=1 size=1024
1010
----
1111
<local fs> create: cold1/000001.blob
1212
<local fs> sync-data: cold1/000001.blob
1313
<local fs> close: cold1/000001.blob
1414

15-
read file-type=blob 1
15+
read file-type=blob file-num=1
1616
0 500
1717
512 1024
1818
----
@@ -24,13 +24,13 @@ size: 1024
2424
512 1024: EOF
2525
<local fs> close: cold1/000001.blob
2626

27-
create 2 local 2 1024
27+
create file-num=2 salt=2 size=1024
2828
----
2929
<local fs> create: p1/000002.sst
3030
<local fs> sync-data: p1/000002.sst
3131
<local fs> close: p1/000002.sst
3232

33-
read 2
33+
read file-num=2
3434
0 500
3535
512 1024
3636
----
@@ -47,27 +47,27 @@ list
4747
000001 -> cold1/000001.blob
4848
000002 -> p1/000002.sst
4949

50-
remove file-type=blob 1
50+
remove file-type=blob file-num=1
5151
----
5252
<local fs> remove: cold1/000001.blob
5353

54-
remove 2
54+
remove file-num=2
5555
----
5656
<local fs> remove: p1/000002.sst
5757

5858
# Verify that we can request cold tier even if it is not configured.
59-
open p2
59+
open dir=p2
6060
----
6161
<local fs> mkdir-all: p2 0755
6262
<local fs> open-dir: p2
6363

64-
create file-type=blob 3 local 3 1024 cold-tier
64+
create file-type=blob file-num=3 cold-tier salt=3 size=1024
6565
----
6666
<local fs> create: p2/000003.blob
6767
<local fs> sync-data: p2/000003.blob
6868
<local fs> close: p2/000003.blob
6969

70-
read file-type=blob 3
70+
read file-type=blob file-num=3
7171
0 500
7272
512 1024
7373
----
@@ -79,6 +79,6 @@ size: 1024
7979
512 1024: EOF
8080
<local fs> close: p2/000003.blob
8181

82-
remove file-type=blob 3
82+
remove file-type=blob file-num=3
8383
----
8484
<local fs> remove: p2/000003.blob

0 commit comments

Comments
 (0)