From 7422a0df1277b665d17a148a97780b7552bc0f1c Mon Sep 17 00:00:00 2001 From: Dave Rolsky Date: Wed, 5 Aug 2026 14:35:25 -0500 Subject: [PATCH] TOOLS-4278 Convert mongorestore metadata and archive tests to testify Converts mongorestore/metadata_test.go and mongorestore_archive_test.go, including the 11 direct t.Fatal/t.Errorf assertions in the two files. The other mongorestore test files stay on GoConvey for now. TestCollectionExists had the only Reset. It was registered inside the "and some test data in a server" block, so it ran on one of the two execution paths, not both. The replacement registers its t.Cleanup in that subtest alone rather than in the shared helper, so the drop still runs on exactly the leaves it used to. That cleanup uses assert, not require. require calls FailNow, which is runtime.Goexit, and that unwinds out of the cleanup runner and skips every cleanup registered earlier -- here it would have stranded the session provider's Close. The Reset it replaces had no such effect, since a failing So still left the surrounding defers to run. TestGetDumpAuthVersion's cases collapse into two tables. That drops 8 call sites while preserving all 15 runtime assertions. 52 runtime assertions before and after. No behavior change. TestReadDumpServerVersionFromArchive still fails locally against a git-built mongod, as it does on master. See the pre-existing issues doc. --- mongorestore/metadata_test.go | 415 +++++++++++----------- mongorestore/mongorestore_archive_test.go | 172 ++++----- 2 files changed, 287 insertions(+), 300 deletions(-) diff --git a/mongorestore/metadata_test.go b/mongorestore/metadata_test.go index 8f613f585..223887ec3 100644 --- a/mongorestore/metadata_test.go +++ b/mongorestore/metadata_test.go @@ -7,6 +7,7 @@ package mongorestore import ( + "context" "fmt" "os" "testing" @@ -17,7 +18,7 @@ import ( commonOpts "github.com/mongodb/mongo-tools/common/options" "github.com/mongodb/mongo-tools/common/testtype" "github.com/mongodb/mongo-tools/common/testutil" - . "github.com/smartystreets/goconvey/convey" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.mongodb.org/mongo-driver/v2/bson" ) @@ -27,9 +28,7 @@ const ExistsDB = "restore_collection_exists" func TestMongoRestoreConnectedToAtlasProxy(t *testing.T) { testtype.SkipUnlessTestType(t, testtype.IntegrationTestType) _, err := testutil.GetBareSession() - if err != nil { - t.Fatalf("No server available") - } + require.NoError(t, err, "must connect to the server") sessionProvider, _, err := testutil.GetBareSessionProvider() require.NoError(t, err) @@ -68,215 +67,229 @@ func TestMongoRestoreConnectedToAtlasProxy(t *testing.T) { func TestCollectionExists(t *testing.T) { testtype.SkipUnlessTestType(t, testtype.IntegrationTestType) _, err := testutil.GetBareSession() - if err != nil { - t.Fatalf("No server available") - } + require.NoError(t, err, "must connect to the server") + + t.Run("collections that exist return true and others return false", func(t *testing.T) { + restore := newCollectionExistsRestore(t) + + session, err := restore.SessionProvider.GetSession() + require.NoError(t, err, "should get a session") + t.Cleanup(func() { + // t.Context() is already canceled by the time cleanups run. + assert.NoError( + t, + session.Database(ExistsDB).Drop(context.Background()), + "should drop the test database", + ) + }) - Convey("With a test mongorestore", t, func() { - sessionProvider, _, err := testutil.GetBareSessionProvider() - So(err, ShouldBeNil) - defer sessionProvider.Close() + _, insertErr := session.Database(ExistsDB). + Collection("one"). + InsertOne(t.Context(), bson.M{}) + require.NoError(t, insertErr, "should insert into collection one") + _, insertErr = session.Database(ExistsDB). + Collection("two"). + InsertOne(t.Context(), bson.M{}) + require.NoError(t, insertErr, "should insert into collection two") + _, insertErr = session.Database(ExistsDB). + Collection("three"). + InsertOne(t.Context(), bson.M{}) + require.NoError(t, insertErr, "should insert into collection three") + + exists, err := restore.CollectionExists(ExistsDB, "one") + require.NoError(t, err, "should check collection one") + assert.True(t, exists, "collection one should exist") + exists, err = restore.CollectionExists(ExistsDB, "two") + require.NoError(t, err, "should check collection two") + assert.True(t, exists, "collection two should exist") + exists, err = restore.CollectionExists(ExistsDB, "three") + require.NoError(t, err, "should check collection three") + assert.True(t, exists, "collection three should exist") + + exists, err = restore.CollectionExists(ExistsDB, "four") + require.NoError(t, err, "should check a collection that was never created") + assert.False(t, exists, "collection four should not exist") + }) - restore := &MongoRestore{ - SessionProvider: sessionProvider, + t.Run("a fake cache is used instead of the server when it exists", func(t *testing.T) { + restore := newCollectionExistsRestore(t) + restore.knownCollections = map[string][]string{ + ExistsDB: {"cats", "dogs", "snakes"}, } + exists, err := restore.CollectionExists(ExistsDB, "dogs") + require.NoError(t, err, "should check a known collection") + assert.True(t, exists, "dogs should be reported present from the known collections cache") + exists, err = restore.CollectionExists(ExistsDB, "two") + require.NoError(t, err, "should check a collection not in the cache") + assert.False( + t, + exists, + "two should not be reported present since it is not in the known collections cache", + ) + }) +} - Convey("and some test data in a server", func() { - session, err := restore.SessionProvider.GetSession() - So(err, ShouldBeNil) - _, insertErr := session.Database(ExistsDB). - Collection("one"). - InsertOne(t.Context(), bson.M{}) - So(insertErr, ShouldBeNil) - _, insertErr = session.Database(ExistsDB). - Collection("two"). - InsertOne(t.Context(), bson.M{}) - So(insertErr, ShouldBeNil) - _, insertErr = session.Database(ExistsDB). - Collection("three"). - InsertOne(t.Context(), bson.M{}) - So(insertErr, ShouldBeNil) - - Convey("collections that exist should return true", func() { - exists, err := restore.CollectionExists(ExistsDB, "one") - So(err, ShouldBeNil) - So(exists, ShouldBeTrue) - exists, err = restore.CollectionExists(ExistsDB, "two") - So(err, ShouldBeNil) - So(exists, ShouldBeTrue) - exists, err = restore.CollectionExists(ExistsDB, "three") - So(err, ShouldBeNil) - So(exists, ShouldBeTrue) - - Convey("and those that do not exist should return false", func() { - exists, err = restore.CollectionExists(ExistsDB, "four") - So(err, ShouldBeNil) - So(exists, ShouldBeFalse) - }) - }) +// newCollectionExistsRestore returns a MongoRestore with a fresh session +// provider and registers a cleanup that closes it, mirroring the original's +// defer sessionProvider.Close(). Dropping ExistsDB is the caller's +// responsibility: only the subtest that inserts data needs to register that +// cleanup, matching the scope of the GoConvey Reset this replaces, which +// only ran on the "and some test data in a server" leaf. +func newCollectionExistsRestore(t *testing.T) *MongoRestore { + t.Helper() - Reset(func() { - err = session.Database(ExistsDB).Drop(t.Context()) - So(err, ShouldBeNil) - }) - }) + sessionProvider, _, err := testutil.GetBareSessionProvider() + require.NoError(t, err, "should get a session provider") + t.Cleanup(sessionProvider.Close) - Convey("and a fake cache should be used instead of the server when it exists", func() { - restore.knownCollections = map[string][]string{ - ExistsDB: {"cats", "dogs", "snakes"}, - } - exists, err := restore.CollectionExists(ExistsDB, "dogs") - So(err, ShouldBeNil) - So(exists, ShouldBeTrue) - exists, err = restore.CollectionExists(ExistsDB, "two") - So(err, ShouldBeNil) - So(exists, ShouldBeFalse) - }) - }) + return &MongoRestore{SessionProvider: sessionProvider} } func TestGetDumpAuthVersion(t *testing.T) { - testtype.SkipUnlessTestType(t, testtype.UnitTestType) - restore := &MongoRestore{} version := db.Version{8, 0, 0} - Convey("With a test mongorestore", t, func() { - Convey("and no --restoreDbUsersAndRoles", func() { - restore = &MongoRestore{ - InputOptions: &InputOptions{}, - ToolOptions: &commonOpts.ToolOptions{}, - NSOptions: &NSOptions{}, - } - Convey("auth version 1 should be detected", func() { - restore.manager = intents.NewIntentManager() - version, err := restore.GetDumpAuthVersion() - So(err, ShouldBeNil) - So(version, ShouldEqual, 1) - }) + t.Run("and no --restoreDbUsersAndRoles", func(t *testing.T) { + cases := []struct { + name string + authVersionFile string + expectedVersion int + }{ + {"auth version 1 should be detected", "", 1}, + {"auth version 3 should be detected", "testdata/auth_version_3.bson", 3}, + {"auth version 5 should be detected", "testdata/auth_version_5.bson", 5}, + } - Convey("auth version 3 should be detected", func() { - restore.manager = intents.NewIntentManager() - intent := &intents.Intent{ - ServerVersion: version, - DB: "admin", - C: "system.version", - Location: "testdata/auth_version_3.bson", + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + restore := &MongoRestore{ + InputOptions: &InputOptions{}, + ToolOptions: &commonOpts.ToolOptions{}, + NSOptions: &NSOptions{}, + manager: intents.NewIntentManager(), } - intent.BSONFile = &realBSONFile{ - path: "testdata/auth_version_3.bson", - intent: intent, + if tc.authVersionFile != "" { + putAuthVersionIntent(restore, version, tc.authVersionFile) } - restore.manager.Put(intent) - version, err := restore.GetDumpAuthVersion() - So(err, ShouldBeNil) - So(version, ShouldEqual, 3) - }) - Convey("auth version 5 should be detected", func() { - restore.manager = intents.NewIntentManager() - intent := &intents.Intent{ - ServerVersion: version, - DB: "admin", - C: "system.version", - Location: "testdata/auth_version_5.bson", - } - intent.BSONFile = &realBSONFile{ - path: "testdata/auth_version_5.bson", - intent: intent, - } - restore.manager.Put(intent) - version, err := restore.GetDumpAuthVersion() - So(err, ShouldBeNil) - So(version, ShouldEqual, 5) + got, err := restore.GetDumpAuthVersion() + require.NoError(t, err, "should determine the auth version") + assert.Equal( + t, + tc.expectedVersion, + got, + "should detect auth version %d", + tc.expectedVersion, + ) }) - }) + } + }) - Convey("using --restoreDbUsersAndRoles", func() { - restore = &MongoRestore{ - InputOptions: &InputOptions{ - RestoreDBUsersAndRoles: true, - }, - ToolOptions: &commonOpts.ToolOptions{ - Namespace: &commonOpts.Namespace{ - DB: "TestDB", - }, - }, - } - - Convey("auth version 3 should be detected when no file exists", func() { - restore.manager = intents.NewIntentManager() - version, err := restore.GetDumpAuthVersion() - So(err, ShouldBeNil) - So(version, ShouldEqual, 3) - }) + t.Run("using --restoreDbUsersAndRoles", func(t *testing.T) { + cases := []struct { + name string + authVersionFile string + expectedVersion int + }{ + {"auth version 3 should be detected when no file exists", "", 3}, + { + "auth version 3 should be detected when a version 3 file exists", + "testdata/auth_version_3.bson", + 3, + }, + {"auth version 5 should be detected", "testdata/auth_version_5.bson", 5}, + } - Convey("auth version 3 should be detected when a version 3 file exists", func() { - restore.manager = intents.NewIntentManager() - intent := &intents.Intent{ - ServerVersion: version, - DB: "admin", - C: "system.version", - Location: "testdata/auth_version_3.bson", + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + restore := newRestoreDBUsersAndRoles() + if tc.authVersionFile != "" { + putAuthVersionIntent(restore, version, tc.authVersionFile) } - intent.BSONFile = &realBSONFile{ - path: "testdata/auth_version_3.bson", - intent: intent, - } - restore.manager.Put(intent) - version, err := restore.GetDumpAuthVersion() - So(err, ShouldBeNil) - So(version, ShouldEqual, 3) - }) - Convey("auth version 5 should be detected", func() { - restore.manager = intents.NewIntentManager() - intent := &intents.Intent{ - ServerVersion: version, - DB: "admin", - C: "system.version", - Location: "testdata/auth_version_5.bson", - } - intent.BSONFile = &realBSONFile{ - path: "testdata/auth_version_5.bson", - intent: intent, - } - restore.manager.Put(intent) - version, err := restore.GetDumpAuthVersion() - So(err, ShouldBeNil) - So(version, ShouldEqual, 5) + got, err := restore.GetDumpAuthVersion() + require.NoError(t, err, "should determine the auth version") + assert.Equal( + t, + tc.expectedVersion, + got, + "should detect auth version %d", + tc.expectedVersion, + ) }) + } - Convey("when system.version does not contain authSchema document", func() { - Convey("should return an error for dump server versions pre 8.1.0", func() { - restore.dumpServerVersion = db.Version{8, 0, 0} - restore.manager = intents.NewIntentManager() - intent := &intents.Intent{ - ServerVersion: version, - DB: "admin", - C: "system.version", - Location: "testdata/system.version.no_auth_schema.bson", - } - intent.BSONFile = &realBSONFile{ - path: "testdata/system.version.no_auth_schema.bson", - intent: intent, - } - restore.manager.Put(intent) - _, err := restore.GetDumpAuthVersion() - So(err, ShouldNotBeNil) - }) - - Convey("auth version 5 should be detected for dump server version 8.1.0+", func() { - restore.dumpServerVersion = db.Version{8, 1, 0} - version, err := restore.GetDumpAuthVersion() - So(err, ShouldBeNil) - So(version, ShouldEqual, 5) - }) - }) - }) + t.Run( + "without an authSchema document should error for dump server versions pre 8.1.0", + func(t *testing.T) { + restore := newRestoreDBUsersAndRoles() + restore.dumpServerVersion = db.Version{8, 0, 0} + putAuthVersionIntent( + restore, + version, + "testdata/system.version.no_auth_schema.bson", + ) + + _, err := restore.GetDumpAuthVersion() + require.Error( + t, + err, + "should reject a dump with no authSchema document below server version 8.1.0", + ) + }, + ) + + t.Run( + "without an authSchema document should detect auth version 5 for dump server version 8.1.0+", + func(t *testing.T) { + restore := newRestoreDBUsersAndRoles() + restore.dumpServerVersion = db.Version{8, 1, 0} + + got, err := restore.GetDumpAuthVersion() + require.NoError(t, err, "should determine the auth version") + assert.Equal( + t, + 5, + got, + "should default to auth version 5 for dump server version 8.1.0+", + ) + }, + ) }) +} +// putAuthVersionIntent adds an intent for testdata/system.version at the +// given location, so GetDumpAuthVersion can read the authSchema document it +// contains. +func putAuthVersionIntent(restore *MongoRestore, version db.Version, location string) { + intent := &intents.Intent{ + ServerVersion: version, + DB: "admin", + C: "system.version", + Location: location, + } + intent.BSONFile = &realBSONFile{ + path: location, + intent: intent, + } + restore.manager.Put(intent) +} + +// newRestoreDBUsersAndRoles builds the MongoRestore fixture shared by the +// "using --restoreDbUsersAndRoles" scenarios. +func newRestoreDBUsersAndRoles() *MongoRestore { + return &MongoRestore{ + InputOptions: &InputOptions{ + RestoreDBUsersAndRoles: true, + }, + ToolOptions: &commonOpts.ToolOptions{ + Namespace: &commonOpts.Namespace{ + DB: "TestDB", + }, + }, + manager: intents.NewIntentManager(), + } } const indexCollationTestDataFile = "testdata/index_collation.json" @@ -285,9 +298,7 @@ func TestIndexGetsSimpleCollation(t *testing.T) { testtype.SkipUnlessTestType(t, testtype.IntegrationTestType) metadata, err := readCollationTestData(indexCollationTestDataFile) - if err != nil { - t.Fatalf("Error reading data file: %v", err) - } + require.NoError(t, err, "should read the collation test data file") dumpDir := testDumpDir{ dirName: "index_collation", @@ -298,22 +309,18 @@ func TestIndexGetsSimpleCollation(t *testing.T) { } err = dumpDir.Create() - if err != nil { - t.Fatalf("Error reading data file: %v", err) - } + require.NoError(t, err, "should create the dump directory") - Convey("With a test MongoRestore", t, func() { - args := []string{ - DropOption, - dumpDir.Path(), - } - restore, err := getRestoreWithArgs(args...) - So(err, ShouldBeNil) - defer restore.Close() + args := []string{ + DropOption, + dumpDir.Path(), + } + restore, err := getRestoreWithArgs(args...) + require.NoError(t, err, "should build a restore instance") + defer restore.Close() - result := restore.Restore() - So(result.Err, ShouldBeNil) - }) + result := restore.Restore() + require.NoError(t, result.Err, "should restore the collection with its simple collation") } func TestAutoIndexIdHandling(t *testing.T) { diff --git a/mongorestore/mongorestore_archive_test.go b/mongorestore/mongorestore_archive_test.go index d2d315023..09465d739 100644 --- a/mongorestore/mongorestore_archive_test.go +++ b/mongorestore/mongorestore_archive_test.go @@ -19,7 +19,7 @@ import ( "github.com/mongodb/mongo-tools/common/testutil" "github.com/mongodb/mongo-tools/mongodump" "github.com/pkg/errors" - . "github.com/smartystreets/goconvey/convey" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.mongodb.org/mongo-driver/v2/bson" "go.mongodb.org/mongo-driver/v2/mongo" @@ -46,108 +46,96 @@ var ( func TestMongorestoreShortArchive(t *testing.T) { testtype.SkipUnlessTestType(t, testtype.IntegrationTestType) _, err := testutil.GetBareSession() - if err != nil { - t.Fatalf("No server available") + require.NoError(t, err, "must connect to the server") + + args := []string{ + ArchiveOption + "=" + testArchive, + NumParallelCollectionsOption, "1", + NumInsertionWorkersOption, "1", + DropOption, } - Convey("With a test MongoRestore", t, func() { - args := []string{ - ArchiveOption + "=" + testArchive, - NumParallelCollectionsOption, "1", - NumInsertionWorkersOption, "1", - DropOption, + file, err := os.Open(testArchive) + require.NotNil(t, file, "should open the test archive") + require.NoError(t, err, "should open the test archive") + + fi, err := file.Stat() + require.NotNil(t, fi, "should stat the test archive") + require.NoError(t, err, "should stat the test archive") + + fileSize := fi.Size() + + for i := fileSize; i >= 0; i -= fileSize / 10 { + log.Logvf( + log.Always, + "Restoring from the first %v bytes of a archive of size %v", + i, + fileSize, + ) + + _, err = file.Seek(0, 0) + require.NoError(t, err, "should seek back to the start of the archive") + + restore, err := getRestoreWithArgs(args...) + require.NoError(t, err, "should build a restore instance") + defer restore.Close() + + restore.archive = &archive.Reader{ + Prelude: &archive.Prelude{}, + In: io.NopCloser(io.LimitReader(file, i)), } - file, err := os.Open(testArchive) - So(file, ShouldNotBeNil) - So(err, ShouldBeNil) - - fi, err := file.Stat() - So(fi, ShouldNotBeNil) - So(err, ShouldBeNil) - - fileSize := fi.Size() - - for i := fileSize; i >= 0; i -= fileSize / 10 { - log.Logvf( - log.Always, - "Restoring from the first %v bytes of a archive of size %v", - i, - fileSize, - ) - - _, err = file.Seek(0, 0) - So(err, ShouldBeNil) - - restore, err := getRestoreWithArgs(args...) - So(err, ShouldBeNil) - defer restore.Close() - - restore.archive = &archive.Reader{ - Prelude: &archive.Prelude{}, - In: io.NopCloser(io.LimitReader(file, i)), - } - - result := restore.Restore() - if i == fileSize { - So(result.Err, ShouldBeNil) - } else { - So(result.Err, ShouldNotBeNil) - } + result := restore.Restore() + if i == fileSize { + require.NoError(t, result.Err, "should restore a complete archive") + } else { + require.Error(t, result.Err, "should error on a truncated archive") } - }) + } } func TestMongorestoreArchiveWithOplog(t *testing.T) { testtype.SkipUnlessTestType(t, testtype.IntegrationTestType) _, err := testutil.GetBareSession() - if err != nil { - t.Fatalf("No server available") - } + require.NoError(t, err, "must connect to the server") - Convey("With a test MongoRestore", t, func() { - args := []string{ - ArchiveOption + "=" + testArchiveWithOplog, - OplogReplayOption, - DropOption, - } - restore, err := getRestoreWithArgs(args...) - So(err, ShouldBeNil) - defer restore.Close() + args := []string{ + ArchiveOption + "=" + testArchiveWithOplog, + OplogReplayOption, + DropOption, + } + restore, err := getRestoreWithArgs(args...) + require.NoError(t, err, "should build a restore instance") + defer restore.Close() - result := restore.Restore() - So(result.Err, ShouldBeNil) - So(result.Failures, ShouldEqual, 0) - So(result.Successes, ShouldNotEqual, 0) - }) + result := restore.Restore() + require.NoError(t, result.Err, "should restore the archive with oplog replay") + assert.EqualValues(t, 0, result.Failures, "should have no restore failures") + assert.NotEqualValues(t, 0, result.Successes, "should have at least one successful restore") } func TestMongorestoreBadFormatArchive(t *testing.T) { testtype.SkipUnlessTestType(t, testtype.IntegrationTestType) _, err := testutil.GetBareSession() - if err != nil { - t.Fatalf("No server available") - } + require.NoError(t, err, "must connect to the server") - Convey("With a test MongoRestore", t, func() { - args := []string{ - ArchiveOption + "=" + testBadFormatArchive, - DropOption, - } - restore, err := getRestoreWithArgs(args...) - So(err, ShouldBeNil) - defer restore.Close() + args := []string{ + ArchiveOption + "=" + testBadFormatArchive, + DropOption, + } + restore, err := getRestoreWithArgs(args...) + require.NoError(t, err, "should build a restore instance") + defer restore.Close() - result := restore.Restore() - Convey( - "A mongorestore on an archive with a bad format should error out instead of hang", - func() { - So(result.Err, ShouldNotBeNil) - So(result.Failures, ShouldEqual, 0) - So(result.Successes, ShouldEqual, 0) - }, - ) - }) + result := restore.Restore() + require.Error(t, result.Err, "should error out instead of hang on a bad format archive") + assert.EqualValues(t, 0, result.Failures, "should report no failures for a bad format archive") + assert.EqualValues( + t, + 0, + result.Successes, + "should report no successes for a bad format archive", + ) } // ---------------------------------------------------------------------- @@ -216,13 +204,9 @@ func testRestoreAdminNamespaces(t *testing.T) { defer func() { err = testDB.Drop(t.Context()) - if err != nil { - t.Fatalf("Failed to drop test database: %v", err) - } + require.NoError(err, "should drop the test database") err = adminSuffixedDB.Drop(t.Context()) - if err != nil { - t.Fatalf("Failed to drop admin suffixed database: %v", err) - } + require.NoError(err, "should drop the admin suffixed database") }() testCases := restoreNamespaceTestCases{ @@ -269,13 +253,9 @@ func testRestoreAdminNamespacesAsAtlasProxy(t *testing.T) { adminSuffixedDB := session.Database(adminSuffixedDBName) defer func() { err = testDB.Drop(t.Context()) - if err != nil { - t.Fatalf("Failed to drop test database: %v", err) - } + require.NoError(err, "should drop the test database") err = adminSuffixedDB.Drop(t.Context()) - if err != nil { - t.Fatalf("Failed to drop admin suffixed database: %v", err) - } + require.NoError(err, "should drop the admin suffixed database") }() testCases := restoreNamespaceTestCases{