From e14337ebef710fe177d15d0be95260af65133784 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Mon, 5 Oct 2026 15:49:31 +0100 Subject: [PATCH] operations: fix index tests failing in the integration tests - Only check the MIME type of the listings on backends which declare both ReadMimeType and WriteMimeType, and ignore its parameters, as many backends report a MIME type they chose themselves or drop the charset. - Write the test file before turning --dry-run on. Chunker finishes an upload with operations.Move, which honours --dry-run, so it left temporary chunks behind which stopped the test directory being removed and made the tests following fail. - Don't check the transfer and delete counts on chunker, as moving its chunks into place is counted in the same stats. - Don't expect a new file to change the time of the directories above it on backends which can't set modification times, as it may get the same time as the files already there. --- fs/operations/index_test.go | 106 +++++++++++++++++++++--------------- 1 file changed, 62 insertions(+), 44 deletions(-) diff --git a/fs/operations/index_test.go b/fs/operations/index_test.go index 56e057e03..fc82d3fca 100644 --- a/fs/operations/index_test.go +++ b/fs/operations/index_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "sort" + "strings" "testing" "time" @@ -56,6 +57,17 @@ func (r *indexRun) index(t *testing.T) (transfers, deletes int64) { return stats.GetTransfers(), stats.GetDeletes() } +// assertCount checks a count of transfers or deletes returned by index +// +// It is not checked on chunker as moving its chunks into place is +// counted in the stats too. +func (r *indexRun) assertCount(t *testing.T, want, got int64, msgAndArgs ...any) { + if strings.HasPrefix(r.Fremote.Name(), "TestChunker") { + return + } + assert.Equal(t, want, got, msgAndArgs...) +} + // read returns the contents of the object at remote func (r *indexRun) read(t *testing.T, remote string) string { o, err := r.Fremote.NewObject(r.ctx, remote) @@ -77,20 +89,22 @@ func (r *indexRun) files(t *testing.T) (names []string) { } // checkMimeType checks the MIME type of the object at remote is want, -// ignoring formatting differences, on backends which store it +// ignoring any parameters, on backends which can both write and read it +// +// The parameters are ignored as many backends don't store them. func (r *indexRun) checkMimeType(t *testing.T, remote, want string) { - o, err := r.Fremote.NewObject(r.ctx, remote) - require.NoError(t, err) - if _, ok := o.(fs.MimeTyper); !ok { + features := r.Fremote.Features() + if !features.ReadMimeType || !features.WriteMimeType { return } - got := fs.MimeType(r.ctx, o) - wantType, wantParams, err := mime.ParseMediaType(want) + o, err := r.Fremote.NewObject(r.ctx, remote) require.NoError(t, err) - gotType, gotParams, err := mime.ParseMediaType(got) + got := fs.MimeType(r.ctx, o) + wantType, _, err := mime.ParseMediaType(want) + require.NoError(t, err) + gotType, _, err := mime.ParseMediaType(got) require.NoError(t, err, got) assert.Equal(t, wantType, gotType) - assert.Equal(t, wantParams, gotParams) } // checkFiles checks the remote contains exactly the objects named @@ -104,8 +118,8 @@ func TestIndex(t *testing.T) { r.opt.Outputs = []string{"index.html=html", "index.json=json", "caddy.json=caddy"} transfers, deletes := r.index(t) - assert.Equal(t, int64(9), transfers) - assert.Equal(t, int64(0), deletes) + r.assertCount(t, 9, transfers) + r.assertCount(t, 0, deletes) r.checkFiles(t, "file1.txt", "index.html", "index.json", "caddy.json", "sub/file2.txt", "sub/index.html", "sub/index.json", "sub/caddy.json", @@ -139,26 +153,26 @@ func TestIndex(t *testing.T) { // A second run changes nothing transfers, deletes = r.index(t) - assert.Equal(t, int64(0), transfers) - assert.Equal(t, int64(0), deletes) + r.assertCount(t, 0, transfers) + r.assertCount(t, 0, deletes) // Unless it is told to rewrite everything r.opt.Rewrite = true transfers, _ = r.index(t) - assert.Equal(t, int64(9), transfers) + r.assertCount(t, 9, transfers) r.opt.Rewrite = false // Adding a file only rewrites the listing of its directory r.WriteObject(r.ctx, "sub/deep/file4.txt", "hello4", t1) transfers, deletes = r.index(t) - assert.Equal(t, int64(3), transfers) - assert.Equal(t, int64(0), deletes) + r.assertCount(t, 3, transfers) + r.assertCount(t, 0, deletes) assert.Contains(t, r.read(t, "sub/deep/index.html"), "file4.txt") // A run with --dry-run changes nothing + r.WriteObject(r.ctx, "sub/deep/file5.txt", "hello5", t1) ctx, ci := fs.AddConfig(r.ctx) ci.DryRun = true - r.WriteObject(ctx, "sub/deep/file5.txt", "hello5", t1) require.NoError(t, operations.Index(ctx, r.Fremote, &r.opt)) assert.NotContains(t, r.read(t, "sub/deep/index.html"), "file5.txt") } @@ -176,15 +190,15 @@ func TestIndexDelete(t *testing.T) { transfers, deletes := r.index(t) if canHaveEmptyDirectories { // The empty directory still exists so keeps its listing - assert.Equal(t, int64(1), transfers) - assert.Equal(t, int64(0), deletes) + r.assertCount(t, 1, transfers) + r.assertCount(t, 0, deletes) r.checkFiles(t, "file1.txt", "index.html", "sub/file2.txt", "sub/index.html", "sub/deep/index.html") assert.Contains(t, r.read(t, "sub/index.html"), "deep/") assert.NotContains(t, r.read(t, "sub/deep/index.html"), "file3.txt") } else { // The directory only existed because of the listing, so it goes - assert.Equal(t, int64(1), transfers) - assert.Equal(t, int64(1), deletes) + r.assertCount(t, 1, transfers) + r.assertCount(t, 1, deletes) r.checkFiles(t, "file1.txt", "index.html", "sub/file2.txt", "sub/index.html") assert.NotContains(t, r.read(t, "sub/index.html"), "deep/") @@ -193,8 +207,8 @@ func TestIndexDelete(t *testing.T) { require.NoError(t, err) require.NoError(t, o.Remove(r.ctx)) transfers, deletes = r.index(t) - assert.Equal(t, int64(1), transfers) - assert.Equal(t, int64(1), deletes) + r.assertCount(t, 1, transfers) + r.assertCount(t, 1, deletes) r.checkFiles(t, "file1.txt", "index.html") assert.NotContains(t, r.read(t, "index.html"), "sub/") } @@ -224,8 +238,8 @@ func TestIndexFilter(t *testing.T) { // A second run changes nothing, in particular it doesn't delete // other/index.html even though other/ has no visible content transfers, deletes := r.index(t) - assert.Equal(t, int64(0), transfers) - assert.Equal(t, int64(0), deletes) + r.assertCount(t, 0, transfers) + r.assertCount(t, 0, deletes) } func TestIndexRules(t *testing.T) { @@ -246,7 +260,7 @@ func TestIndexRules(t *testing.T) { require.NoError(t, err) require.NoError(t, o.Remove(r.ctx)) transfers, _ := r.index(t) - assert.Equal(t, int64(1), transfers) // sub/deep/index.html + r.assertCount(t, 1, transfers) // sub/deep/index.html r.checkFiles(t, "file1.txt", "sub/file2.txt", "sub/index.html", "sub/deep/file3.txt", "sub/deep/index.html") assert.NotEqual(t, "hand written", r.read(t, "sub/deep/index.html")) } @@ -380,12 +394,12 @@ func TestIndexNoHash(t *testing.T) { r.writeFiles() transfers, _ := r.index(t) - assert.Equal(t, int64(3), transfers) + r.assertCount(t, 3, transfers) transfers, _ = r.index(t) - assert.Equal(t, int64(0), transfers) + r.assertCount(t, 0, transfers) r.WriteObject(r.ctx, "sub/file4.txt", "hello4", t1) transfers, _ = r.index(t) - assert.Equal(t, int64(1), transfers) + r.assertCount(t, 1, transfers) assert.Contains(t, r.read(t, "sub/index.html"), "file4.txt") } @@ -399,7 +413,7 @@ func TestIndexChanged(t *testing.T) { r.WriteObject(r.ctx, "other/undeclared.txt", "hello", t1) r.opt.Changed = []string{"sub/deep/file4.txt"} transfers, _ := r.index(t) - assert.Equal(t, int64(1), transfers) + r.assertCount(t, 1, transfers) assert.Contains(t, r.read(t, "sub/deep/index.html"), "file4.txt") assert.NotContains(t, r.read(t, "other/index.html"), "undeclared.txt") @@ -407,7 +421,7 @@ func TestIndexChanged(t *testing.T) { r.WriteObject(r.ctx, "sub/new/file5.txt", "hello5", t1) r.opt.Changed = []string{"sub/new/file5.txt"} transfers, _ = r.index(t) - assert.Equal(t, int64(2), transfers) + r.assertCount(t, 2, transfers) assert.Contains(t, r.read(t, "sub/index.html"), `new/`) assert.Contains(t, r.read(t, "sub/new/index.html"), "file5.txt") @@ -417,7 +431,7 @@ func TestIndexChanged(t *testing.T) { r.WriteObject(r.ctx, name, "hello", t1) r.opt.Changed = []string{changed} transfers, _ = r.index(t) - assert.Equal(t, int64(1), transfers, changed) + r.assertCount(t, 1, transfers, changed) assert.Contains(t, r.read(t, "other/index.html"), name[6:]) } @@ -427,14 +441,14 @@ func TestIndexChanged(t *testing.T) { r.opt.ChangedFrom = []string{filepath.Join(t.TempDir(), "empty.txt")} require.NoError(t, os.WriteFile(r.opt.ChangedFrom[0], nil, 0600)) transfers, _ = r.index(t) - assert.Equal(t, int64(0), transfers) + r.assertCount(t, 0, transfers) assert.NotContains(t, r.read(t, "sub/index.html"), "file6.txt") // Naming the root does a full run r.opt.ChangedFrom = nil r.opt.Changed = []string{"/"} transfers, _ = r.index(t) - assert.Equal(t, int64(1), transfers) + r.assertCount(t, 1, transfers) assert.Contains(t, r.read(t, "sub/index.html"), "file6.txt") // So does exceeding --changed-max-dirs @@ -442,7 +456,7 @@ func TestIndexChanged(t *testing.T) { r.opt.Changed = []string{"other/notes.md"} r.opt.ChangedMaxDirs = 1 transfers, _ = r.index(t) - assert.Equal(t, int64(1), transfers) + r.assertCount(t, 1, transfers) assert.Contains(t, r.read(t, "sub/index.html"), "file7.txt") r.opt.ChangedMaxDirs = 0 @@ -450,7 +464,7 @@ func TestIndexChanged(t *testing.T) { r.WriteObject(r.ctx, "sub/deep/file8.txt", "hello8", t1) r.opt.Changed = []string{"./sub/", "sub//deep/", "sub/deep/"} transfers, _ = r.index(t) - assert.Equal(t, int64(1), transfers) + r.assertCount(t, 1, transfers) assert.Contains(t, r.read(t, "sub/deep/index.html"), "file8.txt") // A partial run doesn't write listings a full run wouldn't @@ -475,20 +489,20 @@ func TestIndexChangedDelete(t *testing.T) { r.opt.Changed = []string{"sub/deep/file3.txt"} transfers, deletes := r.index(t) if r.Fremote.Features().CanHaveEmptyDirectories { - assert.Equal(t, int64(1), transfers) - assert.Equal(t, int64(0), deletes) + r.assertCount(t, 1, transfers) + r.assertCount(t, 0, deletes) assert.NotContains(t, r.read(t, "sub/deep/index.html"), "file3.txt") } else { - assert.Equal(t, int64(1), transfers) - assert.Equal(t, int64(1), deletes) + r.assertCount(t, 1, transfers) + r.assertCount(t, 1, deletes) assert.NotContains(t, r.read(t, "sub/index.html"), "deep/") r.checkFiles(t, "file1.txt", "index.html", "sub/file2.txt", "sub/index.html") // Declaring a directory which has gone is harmless r.opt.Changed = []string{"sub/deep/"} transfers, deletes = r.index(t) - assert.Equal(t, int64(0), transfers) - assert.Equal(t, int64(0), deletes) + r.assertCount(t, 0, transfers) + r.assertCount(t, 0, deletes) } } @@ -505,7 +519,7 @@ func TestIndexChangedFiles(t *testing.T) { require.NoError(t, os.WriteFile(from, []byte("- odd/file8.txt\n\n"), 0600)) r.opt.ChangedFrom = []string{from} transfers, _ := r.index(t) - assert.Equal(t, int64(1), transfers) + r.assertCount(t, 1, transfers) assert.Contains(t, r.read(t, "- odd/index.html"), "file8.txt") assert.NotContains(t, r.read(t, "sub/index.html"), "file9.txt") @@ -515,7 +529,7 @@ func TestIndexChangedFiles(t *testing.T) { r.opt.ChangedFrom = nil r.opt.ChangedCombined = []string{combined} transfers, _ = r.index(t) - assert.Equal(t, int64(1), transfers) + r.assertCount(t, 1, transfers) assert.Contains(t, r.read(t, "sub/index.html"), "file9.txt") // Malformed combined lines are an error @@ -537,7 +551,11 @@ func TestIndexChangedDirTime(t *testing.T) { r.WriteObject(r.ctx, "sub/deep/file4.txt", "hello4", fstest.Time("2030-01-01T00:00:00Z")) r.opt.Changed = []string{"sub/deep/file4.txt"} transfers, _ := r.index(t) - assert.Equal(t, int64(3), transfers) + if precision != fs.ModTimeNotSupported { + // Without settable modification times the new file may not + // change the time of the directories above it + r.assertCount(t, 3, transfers) + } modTimes := r.indexModTimes(t, "index.json") fstest.AssertTimeEqualWithPrecision(t, "sub", fstest.Time("2030-01-01T00:00:00Z"), modTimes["sub"], precision) fstest.AssertTimeEqualWithPrecision(t, "other", t2, modTimes["other"], precision)