Skip to content

Commit a673826

Browse files
guggeroguggero
authored
Merge pull request #9563 from yyforyongyu/fix-unit-test
chanbackup: fix test flake in `TestUpdateAndSwap`
2 parents 97bba57 + 76ade17 commit a673826

File tree

1 file changed

+182
-135
lines changed

1 file changed

+182
-135
lines changed

‎chanbackup/backupfile_test.go

Lines changed: 182 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -45,160 +45,217 @@ func assertFileDeleted(t *testing.T, filePath string) {
4545
}
4646
}
4747

48+
// TestUpdateAndSwapWithArchive test that we're able to properly swap out old
49+
// backups on disk with new ones. In addition, we check for noBackupArchive to
50+
// ensure that the archive file is created when it's set to false, and not
51+
// created when it's set to true.
52+
func TestUpdateAndSwapWithArchive(t *testing.T) {
53+
t.Parallel()
54+
55+
testCases := []struct {
56+
name string
57+
noBackupArchive bool
58+
}{
59+
// Test with noBackupArchive set to true - should not create
60+
// archive.
61+
{
62+
name: "no archive file",
63+
noBackupArchive: true,
64+
},
65+
66+
// Test with noBackupArchive set to false - should create
67+
// archive.
68+
{
69+
name: "with archive file",
70+
noBackupArchive: false,
71+
},
72+
}
73+
74+
for _, tc := range testCases {
75+
t.Run(tc.name, func(t *testing.T) {
76+
tempTestDir := t.TempDir()
77+
78+
fileName := filepath.Join(
79+
tempTestDir, DefaultBackupFileName,
80+
)
81+
tempFileName := filepath.Join(
82+
tempTestDir, DefaultTempBackupFileName,
83+
)
84+
85+
backupFile := NewMultiFile(fileName, tc.noBackupArchive)
86+
87+
// To start with, we'll make a random byte slice that'll
88+
// pose as our packed multi backup.
89+
newPackedMulti, err := makeFakePackedMulti()
90+
require.NoError(t, err)
91+
92+
// With our backup created, we'll now attempt to swap
93+
// out this backup, for the old one.
94+
err = backupFile.UpdateAndSwap(newPackedMulti)
95+
require.NoError(t, err)
96+
97+
// If we read out the file on disk, then it should match
98+
// exactly what we wrote. The temp backup file should
99+
// also be gone.
100+
assertBackupMatches(t, fileName, newPackedMulti)
101+
assertFileDeleted(t, tempFileName)
102+
103+
// Now that we know this is a valid test case, we'll
104+
// make a new packed multi to swap out this current one.
105+
newPackedMulti2, err := makeFakePackedMulti()
106+
require.NoError(t, err)
107+
108+
// We'll then attempt to swap the old version for this
109+
// new one.
110+
err = backupFile.UpdateAndSwap(newPackedMulti2)
111+
require.NoError(t, err)
112+
113+
// Once again, the file written on disk should have been
114+
// properly swapped out with the new instance.
115+
assertBackupMatches(t, fileName, newPackedMulti2)
116+
117+
// Additionally, we shouldn't be able to find the temp
118+
// backup file on disk, as it should be deleted each
119+
// time.
120+
assertFileDeleted(t, tempFileName)
121+
122+
// Now check if archive was created when noBackupArchive
123+
// is false.
124+
archiveDir := filepath.Join(
125+
filepath.Dir(fileName),
126+
DefaultChanBackupArchiveDirName,
127+
)
128+
129+
// When noBackupArchive is true, no new archive file
130+
// should be created.
131+
//
132+
// NOTE: In a real environment, the archive directory
133+
// might exist with older backups before the feature is
134+
// disabled, but for test simplicity (since we clean up
135+
// the directory between test cases), we verify the
136+
// directory doesn't exist at all.
137+
if tc.noBackupArchive {
138+
require.NoDirExists(t, archiveDir)
139+
return
140+
}
141+
142+
// Otherwise we expect an archive to be created.
143+
files, err := os.ReadDir(archiveDir)
144+
require.NoError(t, err)
145+
require.Len(t, files, 1)
146+
147+
// Verify the archive contents match the previous
148+
// backup.
149+
archiveFile := filepath.Join(
150+
archiveDir, files[0].Name(),
151+
)
152+
153+
// The archived content should match the previous backup
154+
// (newPackedMulti) that was just swapped out.
155+
assertBackupMatches(t, archiveFile, newPackedMulti)
156+
157+
// Clean up the archive directory.
158+
os.RemoveAll(archiveDir)
159+
})
160+
}
161+
}
162+
48163
// TestUpdateAndSwap test that we're able to properly swap out old backups on
49164
// disk with new ones. Additionally, after a swap operation succeeds, then each
50165
// time we should only have the main backup file on disk, as the temporary file
51-
// has been removed. Finally, we check for noBackupArchive to ensure that the
52-
// archive file is created when it's set to false, and not created when it's
53-
// set to true.
166+
// has been removed.
54167
func TestUpdateAndSwap(t *testing.T) {
55168
t.Parallel()
56169

57-
tempTestDir := t.TempDir()
58-
59-
testCases := []struct {
60-
fileName string
61-
tempFileName string
170+
// Check that when the main file name is blank, an error is returned.
171+
backupFile := NewMultiFile("", false)
62172

63-
oldTempExists bool
64-
noBackupArchive bool
173+
err := backupFile.UpdateAndSwap(PackedMulti(nil))
174+
require.ErrorIs(t, err, ErrNoBackupFileExists)
65175

66-
valid bool
176+
testCases := []struct {
177+
name string
178+
oldTempExists bool
67179
}{
68-
// Main file name is blank, should fail.
69-
{
70-
fileName: "",
71-
valid: false,
72-
},
73-
74180
// Old temporary file still exists, should be removed. Only one
75181
// file should remain.
76182
{
77-
fileName: filepath.Join(
78-
tempTestDir, DefaultBackupFileName,
79-
),
80-
tempFileName: filepath.Join(
81-
tempTestDir, DefaultTempBackupFileName,
82-
),
183+
name: "remove old temp file",
83184
oldTempExists: true,
84-
valid: true,
85185
},
86186

87187
// Old temp doesn't exist, should swap out file, only a single
88188
// file remains.
89189
{
90-
fileName: filepath.Join(
91-
tempTestDir, DefaultBackupFileName,
92-
),
93-
tempFileName: filepath.Join(
94-
tempTestDir, DefaultTempBackupFileName,
95-
),
96-
valid: true,
190+
name: "swap out file",
191+
oldTempExists: false,
97192
},
193+
}
98194

99-
// Test with noBackupArchive set to true - should not create
100-
// archive.
101-
{
102-
fileName: filepath.Join(
103-
tempTestDir, DefaultBackupFileName,
104-
),
105-
tempFileName: filepath.Join(
106-
tempTestDir, DefaultTempBackupFileName,
107-
),
108-
noBackupArchive: true,
109-
valid: true,
110-
},
195+
for _, tc := range testCases {
196+
t.Run(tc.name, func(t *testing.T) {
197+
tempTestDir := t.TempDir()
111198

112-
// Test with v set to false - should create
113-
// archive.
114-
{
115-
fileName: filepath.Join(
199+
fileName := filepath.Join(
116200
tempTestDir, DefaultBackupFileName,
117-
),
118-
tempFileName: filepath.Join(
201+
)
202+
tempFileName := filepath.Join(
119203
tempTestDir, DefaultTempBackupFileName,
120-
),
121-
noBackupArchive: false,
122-
valid: true,
123-
},
124-
}
125-
for i, testCase := range testCases {
126-
backupFile := NewMultiFile(
127-
testCase.fileName, testCase.noBackupArchive,
128-
)
204+
)
129205

130-
// To start with, we'll make a random byte slice that'll pose
131-
// as our packed multi backup.
132-
newPackedMulti, err := makeFakePackedMulti()
133-
if err != nil {
134-
t.Fatalf("unable to make test backup: %v", err)
135-
}
206+
backupFile := NewMultiFile(fileName, false)
136207

137-
// If the old temporary file is meant to exist, then we'll
138-
// create it now as an empty file.
139-
if testCase.oldTempExists {
140-
f, err := os.Create(testCase.tempFileName)
141-
if err != nil {
142-
t.Fatalf("unable to create temp file: %v", err)
143-
}
144-
require.NoError(t, f.Close())
208+
// To start with, we'll make a random byte slice that'll
209+
// pose as our packed multi backup.
210+
newPackedMulti, err := makeFakePackedMulti()
211+
require.NoError(t, err)
145212

146-
// TODO(roasbeef): mock out fs calls?
147-
}
213+
// If the old temporary file is meant to exist, then
214+
// we'll create it now as an empty file.
215+
if tc.oldTempExists {
216+
f, err := os.Create(tempFileName)
217+
require.NoError(t, err)
218+
require.NoError(t, f.Close())
148219

149-
// With our backup created, we'll now attempt to swap out this
150-
// backup, for the old one.
151-
err = backupFile.UpdateAndSwap(PackedMulti(newPackedMulti))
152-
switch {
153-
// If this is a valid test case, and we failed, then we'll
154-
// return an error.
155-
case err != nil && testCase.valid:
156-
t.Fatalf("#%v, unable to swap file: %v", i, err)
220+
// TODO(roasbeef): mock out fs calls?
221+
}
157222

158-
// If this is an invalid test case, and we passed it, then
159-
// we'll return an error.
160-
case err == nil && !testCase.valid:
161-
t.Fatalf("#%v file swap should have failed: %v", i, err)
162-
}
223+
// With our backup created, we'll now attempt to swap
224+
// out this backup, for the old one.
225+
err = backupFile.UpdateAndSwap(newPackedMulti)
226+
require.NoError(t, err)
163227

164-
if !testCase.valid {
165-
continue
166-
}
228+
// If we read out the file on disk, then it should match
229+
// exactly what we wrote. The temp backup file should
230+
// also be gone.
231+
assertBackupMatches(t, fileName, newPackedMulti)
232+
assertFileDeleted(t, tempFileName)
167233

168-
// If we read out the file on disk, then it should match
169-
// exactly what we wrote. The temp backup file should also be
170-
// gone.
171-
assertBackupMatches(t, testCase.fileName, newPackedMulti)
172-
assertFileDeleted(t, testCase.tempFileName)
234+
// Now that we know this is a valid test case, we'll
235+
// make a new packed multi to swap out this current one.
236+
newPackedMulti2, err := makeFakePackedMulti()
237+
require.NoError(t, err)
173238

174-
// Now that we know this is a valid test case, we'll make a new
175-
// packed multi to swap out this current one.
176-
newPackedMulti2, err := makeFakePackedMulti()
177-
if err != nil {
178-
t.Fatalf("unable to make test backup: %v", err)
179-
}
239+
// We'll then attempt to swap the old version for this
240+
// new one.
241+
err = backupFile.UpdateAndSwap(newPackedMulti2)
242+
require.NoError(t, err)
180243

181-
// We'll then attempt to swap the old version for this new one.
182-
err = backupFile.UpdateAndSwap(PackedMulti(newPackedMulti2))
183-
if err != nil {
184-
t.Fatalf("unable to swap file: %v", err)
185-
}
244+
// Once again, the file written on disk should have been
245+
// properly swapped out with the new instance.
246+
assertBackupMatches(t, fileName, newPackedMulti2)
186247

187-
// Once again, the file written on disk should have been
188-
// properly swapped out with the new instance.
189-
assertBackupMatches(t, testCase.fileName, newPackedMulti2)
190-
191-
// Additionally, we shouldn't be able to find the temp backup
192-
// file on disk, as it should be deleted each time.
193-
assertFileDeleted(t, testCase.tempFileName)
194-
195-
// Now check if archive was created when noBackupArchive is
196-
// false.
197-
archiveDir := filepath.Join(
198-
filepath.Dir(testCase.fileName),
199-
DefaultChanBackupArchiveDirName,
200-
)
201-
if !testCase.noBackupArchive {
248+
// Additionally, we shouldn't be able to find the temp
249+
// backup file on disk, as it should be deleted each
250+
// time.
251+
assertFileDeleted(t, tempFileName)
252+
253+
// Now check if archive was created when noBackupArchive
254+
// is false.
255+
archiveDir := filepath.Join(
256+
filepath.Dir(fileName),
257+
DefaultChanBackupArchiveDirName,
258+
)
202259
files, err := os.ReadDir(archiveDir)
203260
require.NoError(t, err)
204261
require.Len(t, files, 1)
@@ -208,24 +265,14 @@ func TestUpdateAndSwap(t *testing.T) {
208265
archiveFile := filepath.Join(
209266
archiveDir, files[0].Name(),
210267
)
211-
// The archived content should match the previous
212-
// backup (newPackedMulti) that was just swapped out.
268+
269+
// The archived content should match the previous backup
270+
// (newPackedMulti) that was just swapped out.
213271
assertBackupMatches(t, archiveFile, newPackedMulti)
214272

215273
// Clean up the archive directory.
216274
os.RemoveAll(archiveDir)
217-
218-
continue
219-
}
220-
221-
// When noBackupArchive is true, no new archive file should be
222-
// created. Note: In a real environment, the archive directory
223-
// might exist with older backups before the feature is
224-
// disabled, but for test simplicity (since we clean up the
225-
// directory between test cases), we verify the directory
226-
// doesn't exist at all.
227-
require.NoDirExists(t, archiveDir)
228-
275+
})
229276
}
230277
}
231278

0 commit comments

Comments
 (0)