From 274e85da596e4e2cbc42bc9d6cf5a09324943b13 Mon Sep 17 00:00:00 2001 From: Matt Dale <9760375+matthewdale@users.noreply.github.com> Date: Wed, 27 Aug 2025 12:27:38 -0700 Subject: [PATCH 01/12] Split TestCursor into separate test functions. --- internal/integration/cursor_test.go | 476 ++++++++++++++-------------- 1 file changed, 244 insertions(+), 232 deletions(-) diff --git a/internal/integration/cursor_test.go b/internal/integration/cursor_test.go index 8bba46df53..4177336540 100644 --- a/internal/integration/cursor_test.go +++ b/internal/integration/cursor_test.go @@ -29,11 +29,8 @@ const ( func TestCursor(t *testing.T) { mt := mtest.New(t, mtest.NewOptions().CreateClient(false)) - cappedCollectionOpts := options.CreateCollection().SetCapped(true).SetSizeInBytes(64 * 1024) - // Server versions 2.6 and 3.0 use OP_GET_MORE so this works on >= 3.2 and when RequireAPIVersion is false; - // getMore cannot be sent with RunCommand as server API options will be attached when they should not be. - mt.RunOpts("cursor is killed on server", mtest.NewOptions().MinServerVersion("3.2").RequireAPIVersion(false), func(mt *mtest.T) { + mt.Run("cursor is killed on server", func(mt *mtest.T) { initCollection(mt, mt.Coll) c, err := mt.Coll.Find(context.Background(), bson.D{}, options.Find().SetBatchSize(2)) assert.Nil(mt, err, "Find error: %v", err) @@ -50,235 +47,8 @@ func TestCursor(t *testing.T) { ce := err.(mongo.CommandError) assert.Equal(mt, int32(errorCursorNotFound), ce.Code, "expected error code %v, got %v", errorCursorNotFound, ce.Code) }) - mt.RunOpts("try next", noClientOpts, func(mt *mtest.T) { - // Skip tests if running against serverless, as capped collections are banned. - if os.Getenv("SERVERLESS") == "serverless" { - mt.Skip("skipping as serverless forbids capped collections") - } - - mt.Run("existing non-empty batch", func(mt *mtest.T) { - // If there's already documents in the current batch, TryNext should return true without doing a getMore - - initCollection(mt, mt.Coll) - cursor, err := mt.Coll.Find(context.Background(), bson.D{}) - assert.Nil(mt, err, "Find error: %v", err) - defer cursor.Close(context.Background()) - tryNextExistingBatchTest(mt, cursor) - }) - mt.RunOpts("one getMore sent", mtest.NewOptions().CollectionCreateOptions(cappedCollectionOpts), func(mt *mtest.T) { - // If the current batch is empty, TryNext should send one getMore and return. - - // insert a document because a tailable cursor will only have a non-zero ID if the initial Find matches - // at least one document - _, err := mt.Coll.InsertOne(context.Background(), bson.D{{"x", 1}}) - assert.Nil(mt, err, "InsertOne error: %v", err) - - cursor, err := mt.Coll.Find(context.Background(), bson.D{}, options.Find().SetCursorType(options.Tailable)) - assert.Nil(mt, err, "Find error: %v", err) - defer cursor.Close(context.Background()) - - // first call to TryNext should return 1 document - assert.True(mt, cursor.TryNext(context.Background()), "expected Next to return true, got false") - // TryNext should attempt one getMore - mt.ClearEvents() - assert.False(mt, cursor.TryNext(context.Background()), "unexpected document %v", cursor.Current) - verifyOneGetmoreSent(mt) - }) - mt.RunOpts("getMore error", mtest.NewOptions().ClientType(mtest.Mock), func(mt *mtest.T) { - findRes := mtest.CreateCursorResponse(50, "foo.bar", mtest.FirstBatch) - mt.AddMockResponses(findRes) - cursor, err := mt.Coll.Find(context.Background(), bson.D{}) - assert.Nil(mt, err, "Find error: %v", err) - defer cursor.Close(context.Background()) - tryNextGetmoreError(mt, cursor) - }) - }) - mt.RunOpts("RemainingBatchLength", noClientOpts, func(mt *mtest.T) { - cappedMtOpts := mtest.NewOptions().CollectionCreateOptions(cappedCollectionOpts) - // Skip tests if running against serverless, as capped collections are banned. - if os.Getenv("SERVERLESS") == "serverless" { - mt.Skip("skipping as serverless forbids capped collections") - } - - mt.RunOpts("first batch is non empty", cappedMtOpts, func(mt *mtest.T) { - // Test that the cursor reports the correct value for RemainingBatchLength at various execution points if - // the first batch from the server is non-empty. - - initCollection(mt, mt.Coll) - - // Create a tailable await cursor with a low cursor timeout. - batchSize := 2 - findOpts := options.Find(). - SetBatchSize(int32(batchSize)). - SetCursorType(options.TailableAwait). - SetMaxAwaitTime(100 * time.Millisecond) - cursor, err := mt.Coll.Find(context.Background(), bson.D{}, findOpts) - assert.Nil(mt, err, "Find error: %v", err) - defer cursor.Close(context.Background()) - - mt.ClearEvents() - - // The initial batch length should be equal to the batchSize. Do batchSize Next calls to exhaust the current - // batch and assert that no getMore was done. - assertCursorBatchLength(mt, cursor, batchSize) - for i := 0; i < batchSize; i++ { - prevLength := cursor.RemainingBatchLength() - if !cursor.Next(context.Background()) { - mt.Fatalf("expected Next to return true on index %d; cursor err: %v", i, cursor.Err()) - } - - // Each successful Next call should decrement batch length by 1. - assertCursorBatchLength(mt, cursor, prevLength-1) - } - evt := mt.GetStartedEvent() - assert.Nil(mt, evt, "expected no events, got %v", evt) - - // The batch is exhausted, so the batch length should be 0. Do one Next call, which should do a getMore and - // fetch batchSize more documents. The batch length after the call should be (batchSize-1) because Next consumes - // one document. - assertCursorBatchLength(mt, cursor, 0) - - assert.True(mt, cursor.Next(context.Background()), "expected Next to return true; cursor err: %v", cursor.Err()) - evt = mt.GetStartedEvent() - assert.NotNil(mt, evt, "expected CommandStartedEvent, got nil") - assert.Equal(mt, "getMore", evt.CommandName, "expected command %q, got %q", "getMore", evt.CommandName) - - assertCursorBatchLength(mt, cursor, batchSize-1) - }) - mt.RunOpts("first batch is empty", mtest.NewOptions().ClientType(mtest.Mock), func(mt *mtest.T) { - // Test that the cursor reports the correct value for RemainingBatchLength if the first batch is empty. - // Using a mock deployment simplifies this test because the server won't create a valid cursor if the - // collection is empty when the find is run. - - cursorID := int64(50) - ns := mt.DB.Name() + "." + mt.Coll.Name() - getMoreBatch := []bson.D{ - {{"x", 1}}, - {{"x", 2}}, - } - - // Create mock responses. - find := mtest.CreateCursorResponse(cursorID, ns, mtest.FirstBatch) - getMore := mtest.CreateCursorResponse(cursorID, ns, mtest.NextBatch, getMoreBatch...) - killCursors := mtest.CreateSuccessResponse() - mt.AddMockResponses(find, getMore, killCursors) - - cursor, err := mt.Coll.Find(context.Background(), bson.D{}) - assert.Nil(mt, err, "Find error: %v", err) - defer cursor.Close(context.Background()) - mt.ClearEvents() - - for { - if cursor.TryNext(context.Background()) { - break - } - - assert.Nil(mt, cursor.Err(), "cursor error: %v", err) - assertCursorBatchLength(mt, cursor, 0) - } - // TryNext consumes one document so the remaining batch size should be len(getMoreBatch)-1. - assertCursorBatchLength(mt, cursor, len(getMoreBatch)-1) - }) - }) - mt.RunOpts("all", noClientOpts, func(mt *mtest.T) { - failpointOpts := mtest.NewOptions().Topologies(mtest.ReplicaSet).MinServerVersion("4.0") - mt.RunOpts("getMore error", failpointOpts, func(mt *mtest.T) { - failpointData := failpoint.Data{ - FailCommands: []string{"getMore"}, - ErrorCode: 100, - } - mt.SetFailPoint(failpoint.FailPoint{ - ConfigureFailPoint: "failCommand", - Mode: failpoint.ModeAlwaysOn, - Data: failpointData, - }) - initCollection(mt, mt.Coll) - cursor, err := mt.Coll.Find(context.Background(), bson.D{}, options.Find().SetBatchSize(2)) - assert.Nil(mt, err, "Find error: %v", err) - defer cursor.Close(context.Background()) - - var docs []bson.D - err = cursor.All(context.Background(), &docs) - assert.NotNil(mt, err, "expected change stream error, got nil") - - // make sure that a mongo.CommandError is returned instead of a driver.Error - mongoErr, ok := err.(mongo.CommandError) - assert.True(mt, ok, "expected mongo.CommandError, got: %T", err) - assert.Equal(mt, failpointData.ErrorCode, mongoErr.Code, "expected code %v, got: %v", failpointData.ErrorCode, mongoErr.Code) - }) - mt.Run("deferred Close uses context.Background", func(mt *mtest.T) { - initCollection(mt, mt.Coll) - - // Find with batchSize 2 so All will run getMore for next 3 docs and error. - cur, err := mt.Coll.Find(context.Background(), bson.D{}, - options.Find().SetBatchSize(2)) - assert.Nil(mt, err, "Find error: %v", err) - - // Create a context and immediately cancel it. - canceledCtx, cancel := context.WithCancel(context.Background()) - cancel() - - // Clear "insert" and "find" events. - mt.ClearEvents() - - // Call All with the canceled context and expect context.Canceled. - var docs []bson.D - err = cur.All(canceledCtx, &docs) - assert.NotNil(mt, err, "expected error for All, got nil") - assert.True(mt, errors.Is(err, context.Canceled), - "expected context.Canceled error, got %v", err) - - // Assert that a "getMore" command was sent and failed (Next used the - // canceled context). - stEvt := mt.GetStartedEvent() - assert.NotNil(mt, stEvt, `expected a "getMore" started event, got no event`) - assert.Equal(mt, stEvt.CommandName, "getMore", - `expected a "getMore" started event, got %q`, stEvt.CommandName) - fEvt := mt.GetFailedEvent() - assert.NotNil(mt, fEvt, `expected a failed "getMore" event, got no event`) - assert.Equal(mt, fEvt.CommandName, "getMore", - `expected a failed "getMore" event, got %q`, fEvt.CommandName) - - // Assert that a "killCursors" command was sent and was successful (Close - // used the 2 second Client Timeout). - stEvt = mt.GetStartedEvent() - assert.NotNil(mt, stEvt, `expected a "killCursors" started event, got no event`) - assert.Equal(mt, stEvt.CommandName, "killCursors", - `expected a "killCursors" started event, got %q`, stEvt.CommandName) - suEvt := mt.GetSucceededEvent() - assert.NotNil(mt, suEvt, `expected a successful "killCursors" event, got no event`) - assert.Equal(mt, suEvt.CommandName, "killCursors", - `expected a successful "killCursors" event, got %q`, suEvt.CommandName) - }) - }) - mt.RunOpts("close", noClientOpts, func(mt *mtest.T) { - failpointOpts := mtest.NewOptions().Topologies(mtest.ReplicaSet).MinServerVersion("4.0") - mt.RunOpts("killCursors error", failpointOpts, func(mt *mtest.T) { - failpointData := failpoint.Data{ - FailCommands: []string{"killCursors"}, - ErrorCode: 100, - } - mt.SetFailPoint(failpoint.FailPoint{ - ConfigureFailPoint: "failCommand", - Mode: failpoint.ModeAlwaysOn, - Data: failpointData, - }) - initCollection(mt, mt.Coll) - cursor, err := mt.Coll.Find(context.Background(), bson.D{}, options.Find().SetBatchSize(2)) - assert.Nil(mt, err, "Find error: %v", err) - - err = cursor.Close(context.Background()) - assert.NotNil(mt, err, "expected change stream error, got nil") - - // make sure that a mongo.CommandError is returned instead of a driver.Error - mongoErr, ok := err.(mongo.CommandError) - assert.True(mt, ok, "expected mongo.CommandError, got: %T", err) - assert.Equal(mt, failpointData.ErrorCode, mongoErr.Code, "expected code %v, got: %v", failpointData.ErrorCode, mongoErr.Code) - }) - }) - // For versions < 3.2, the first find will get all the documents - mt.RunOpts("set batchSize", mtest.NewOptions().MinServerVersion("3.2"), func(mt *mtest.T) { + mt.Run("set batchSize", func(mt *mtest.T) { initCollection(mt, mt.Coll) mt.ClearEvents() @@ -307,6 +77,248 @@ func TestCursor(t *testing.T) { }) } +func TestCursor_TryNext(t *testing.T) { + mt := mtest.New(t, mtest.NewOptions().CreateClient(false)) + + // Skip tests if running against serverless, as capped collections are banned. + if os.Getenv("SERVERLESS") == "serverless" { + mt.Skip("skipping as serverless forbids capped collections") + } + + mt.Run("existing non-empty batch", func(mt *mtest.T) { + // If there's already documents in the current batch, TryNext should return true without doing a getMore + + initCollection(mt, mt.Coll) + cursor, err := mt.Coll.Find(context.Background(), bson.D{}) + assert.Nil(mt, err, "Find error: %v", err) + defer cursor.Close(context.Background()) + tryNextExistingBatchTest(mt, cursor) + }) + + cappedCollectionOpts := options.CreateCollection().SetCapped(true).SetSizeInBytes(64 * 1024) + mt.RunOpts("one getMore sent", mtest.NewOptions().CollectionCreateOptions(cappedCollectionOpts), func(mt *mtest.T) { + // If the current batch is empty, TryNext should send one getMore and return. + + // insert a document because a tailable cursor will only have a non-zero ID if the initial Find matches + // at least one document + _, err := mt.Coll.InsertOne(context.Background(), bson.D{{"x", 1}}) + assert.Nil(mt, err, "InsertOne error: %v", err) + + cursor, err := mt.Coll.Find(context.Background(), bson.D{}, options.Find().SetCursorType(options.Tailable)) + assert.Nil(mt, err, "Find error: %v", err) + defer cursor.Close(context.Background()) + + // first call to TryNext should return 1 document + assert.True(mt, cursor.TryNext(context.Background()), "expected Next to return true, got false") + // TryNext should attempt one getMore + mt.ClearEvents() + assert.False(mt, cursor.TryNext(context.Background()), "unexpected document %v", cursor.Current) + verifyOneGetmoreSent(mt) + }) + mt.RunOpts("getMore error", mtest.NewOptions().ClientType(mtest.Mock), func(mt *mtest.T) { + findRes := mtest.CreateCursorResponse(50, "foo.bar", mtest.FirstBatch) + mt.AddMockResponses(findRes) + cursor, err := mt.Coll.Find(context.Background(), bson.D{}) + assert.Nil(mt, err, "Find error: %v", err) + defer cursor.Close(context.Background()) + tryNextGetmoreError(mt, cursor) + }) +} + +func TestCursor_RemainingBatchLength(t *testing.T) { + mt := mtest.New(t, mtest.NewOptions().CreateClient(false)) + + cappedCollectionOpts := options.CreateCollection().SetCapped(true).SetSizeInBytes(64 * 1024) + cappedMtOpts := mtest.NewOptions().CollectionCreateOptions(cappedCollectionOpts) + // Skip tests if running against serverless, as capped collections are banned. + if os.Getenv("SERVERLESS") == "serverless" { + mt.Skip("skipping as serverless forbids capped collections") + } + + mt.RunOpts("first batch is non empty", cappedMtOpts, func(mt *mtest.T) { + // Test that the cursor reports the correct value for RemainingBatchLength at various execution points if + // the first batch from the server is non-empty. + + initCollection(mt, mt.Coll) + + // Create a tailable await cursor with a low cursor timeout. + batchSize := 2 + findOpts := options.Find(). + SetBatchSize(int32(batchSize)). + SetCursorType(options.TailableAwait). + SetMaxAwaitTime(100 * time.Millisecond) + cursor, err := mt.Coll.Find(context.Background(), bson.D{}, findOpts) + assert.Nil(mt, err, "Find error: %v", err) + defer cursor.Close(context.Background()) + + mt.ClearEvents() + + // The initial batch length should be equal to the batchSize. Do batchSize Next calls to exhaust the current + // batch and assert that no getMore was done. + assertCursorBatchLength(mt, cursor, batchSize) + for i := 0; i < batchSize; i++ { + prevLength := cursor.RemainingBatchLength() + if !cursor.Next(context.Background()) { + mt.Fatalf("expected Next to return true on index %d; cursor err: %v", i, cursor.Err()) + } + + // Each successful Next call should decrement batch length by 1. + assertCursorBatchLength(mt, cursor, prevLength-1) + } + evt := mt.GetStartedEvent() + assert.Nil(mt, evt, "expected no events, got %v", evt) + + // The batch is exhausted, so the batch length should be 0. Do one Next call, which should do a getMore and + // fetch batchSize more documents. The batch length after the call should be (batchSize-1) because Next consumes + // one document. + assertCursorBatchLength(mt, cursor, 0) + + assert.True(mt, cursor.Next(context.Background()), "expected Next to return true; cursor err: %v", cursor.Err()) + evt = mt.GetStartedEvent() + assert.NotNil(mt, evt, "expected CommandStartedEvent, got nil") + assert.Equal(mt, "getMore", evt.CommandName, "expected command %q, got %q", "getMore", evt.CommandName) + + assertCursorBatchLength(mt, cursor, batchSize-1) + }) + mt.RunOpts("first batch is empty", mtest.NewOptions().ClientType(mtest.Mock), func(mt *mtest.T) { + // Test that the cursor reports the correct value for RemainingBatchLength if the first batch is empty. + // Using a mock deployment simplifies this test because the server won't create a valid cursor if the + // collection is empty when the find is run. + + cursorID := int64(50) + ns := mt.DB.Name() + "." + mt.Coll.Name() + getMoreBatch := []bson.D{ + {{"x", 1}}, + {{"x", 2}}, + } + + // Create mock responses. + find := mtest.CreateCursorResponse(cursorID, ns, mtest.FirstBatch) + getMore := mtest.CreateCursorResponse(cursorID, ns, mtest.NextBatch, getMoreBatch...) + killCursors := mtest.CreateSuccessResponse() + mt.AddMockResponses(find, getMore, killCursors) + + cursor, err := mt.Coll.Find(context.Background(), bson.D{}) + assert.Nil(mt, err, "Find error: %v", err) + defer cursor.Close(context.Background()) + mt.ClearEvents() + + for { + if cursor.TryNext(context.Background()) { + break + } + + assert.Nil(mt, cursor.Err(), "cursor error: %v", err) + assertCursorBatchLength(mt, cursor, 0) + } + // TryNext consumes one document so the remaining batch size should be len(getMoreBatch)-1. + assertCursorBatchLength(mt, cursor, len(getMoreBatch)-1) + }) +} + +func TestCursor_All(t *testing.T) { + mt := mtest.New(t, mtest.NewOptions().CreateClient(false)) + + failpointOpts := mtest.NewOptions().Topologies(mtest.ReplicaSet).MinServerVersion("4.0") + mt.RunOpts("getMore error", failpointOpts, func(mt *mtest.T) { + failpointData := failpoint.Data{ + FailCommands: []string{"getMore"}, + ErrorCode: 100, + } + mt.SetFailPoint(failpoint.FailPoint{ + ConfigureFailPoint: "failCommand", + Mode: failpoint.ModeAlwaysOn, + Data: failpointData, + }) + initCollection(mt, mt.Coll) + cursor, err := mt.Coll.Find(context.Background(), bson.D{}, options.Find().SetBatchSize(2)) + assert.Nil(mt, err, "Find error: %v", err) + defer cursor.Close(context.Background()) + + var docs []bson.D + err = cursor.All(context.Background(), &docs) + assert.NotNil(mt, err, "expected change stream error, got nil") + + // make sure that a mongo.CommandError is returned instead of a driver.Error + mongoErr, ok := err.(mongo.CommandError) + assert.True(mt, ok, "expected mongo.CommandError, got: %T", err) + assert.Equal(mt, failpointData.ErrorCode, mongoErr.Code, "expected code %v, got: %v", failpointData.ErrorCode, mongoErr.Code) + }) + + mt.Run("deferred Close uses context.Background", func(mt *mtest.T) { + initCollection(mt, mt.Coll) + + // Find with batchSize 2 so All will run getMore for next 3 docs and error. + cur, err := mt.Coll.Find(context.Background(), bson.D{}, + options.Find().SetBatchSize(2)) + assert.Nil(mt, err, "Find error: %v", err) + + // Create a context and immediately cancel it. + canceledCtx, cancel := context.WithCancel(context.Background()) + cancel() + + // Clear "insert" and "find" events. + mt.ClearEvents() + + // Call All with the canceled context and expect context.Canceled. + var docs []bson.D + err = cur.All(canceledCtx, &docs) + assert.NotNil(mt, err, "expected error for All, got nil") + assert.True(mt, errors.Is(err, context.Canceled), + "expected context.Canceled error, got %v", err) + + // Assert that a "getMore" command was sent and failed (Next used the + // canceled context). + stEvt := mt.GetStartedEvent() + assert.NotNil(mt, stEvt, `expected a "getMore" started event, got no event`) + assert.Equal(mt, stEvt.CommandName, "getMore", + `expected a "getMore" started event, got %q`, stEvt.CommandName) + fEvt := mt.GetFailedEvent() + assert.NotNil(mt, fEvt, `expected a failed "getMore" event, got no event`) + assert.Equal(mt, fEvt.CommandName, "getMore", + `expected a failed "getMore" event, got %q`, fEvt.CommandName) + + // Assert that a "killCursors" command was sent and was successful (Close + // used the 2 second Client Timeout). + stEvt = mt.GetStartedEvent() + assert.NotNil(mt, stEvt, `expected a "killCursors" started event, got no event`) + assert.Equal(mt, stEvt.CommandName, "killCursors", + `expected a "killCursors" started event, got %q`, stEvt.CommandName) + suEvt := mt.GetSucceededEvent() + assert.NotNil(mt, suEvt, `expected a successful "killCursors" event, got no event`) + assert.Equal(mt, suEvt.CommandName, "killCursors", + `expected a successful "killCursors" event, got %q`, suEvt.CommandName) + }) +} + +func TestCursor_Close(t *testing.T) { + mt := mtest.New(t, mtest.NewOptions().CreateClient(false)) + + failpointOpts := mtest.NewOptions().Topologies(mtest.ReplicaSet).MinServerVersion("4.0") + mt.RunOpts("killCursors error", failpointOpts, func(mt *mtest.T) { + failpointData := failpoint.Data{ + FailCommands: []string{"killCursors"}, + ErrorCode: 100, + } + mt.SetFailPoint(failpoint.FailPoint{ + ConfigureFailPoint: "failCommand", + Mode: failpoint.ModeAlwaysOn, + Data: failpointData, + }) + initCollection(mt, mt.Coll) + cursor, err := mt.Coll.Find(context.Background(), bson.D{}, options.Find().SetBatchSize(2)) + assert.Nil(mt, err, "Find error: %v", err) + + err = cursor.Close(context.Background()) + assert.NotNil(mt, err, "expected change stream error, got nil") + + // make sure that a mongo.CommandError is returned instead of a driver.Error + mongoErr, ok := err.(mongo.CommandError) + assert.True(mt, ok, "expected mongo.CommandError, got: %T", err) + assert.Equal(mt, failpointData.ErrorCode, mongoErr.Code, "expected code %v, got: %v", failpointData.ErrorCode, mongoErr.Code) + }) +} + func parseMaxAwaitTime(mt *mtest.T, evt *event.CommandStartedEvent) int64 { mt.Helper() From 94909f809cfd792e6d52f8014f8f446a2d0df7b3 Mon Sep 17 00:00:00 2001 From: Matt Dale <9760375+matthewdale@users.noreply.github.com> Date: Fri, 29 Aug 2025 14:58:32 -0700 Subject: [PATCH 02/12] Apply suggestion from @prestonvasquez Co-authored-by: Preston Vasquez --- internal/integration/cursor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/integration/cursor_test.go b/internal/integration/cursor_test.go index 4177336540..ea36267d3c 100644 --- a/internal/integration/cursor_test.go +++ b/internal/integration/cursor_test.go @@ -90,7 +90,7 @@ func TestCursor_TryNext(t *testing.T) { initCollection(mt, mt.Coll) cursor, err := mt.Coll.Find(context.Background(), bson.D{}) - assert.Nil(mt, err, "Find error: %v", err) + require.NoError(mt, err, "Find error: %v", err) defer cursor.Close(context.Background()) tryNextExistingBatchTest(mt, cursor) }) From 0d141b4dc7c9586fbdb3531b6fd35e4b6422bf69 Mon Sep 17 00:00:00 2001 From: Matt Dale <9760375+matthewdale@users.noreply.github.com> Date: Fri, 29 Aug 2025 14:58:42 -0700 Subject: [PATCH 03/12] Apply suggestion from @prestonvasquez Co-authored-by: Preston Vasquez --- internal/integration/cursor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/integration/cursor_test.go b/internal/integration/cursor_test.go index ea36267d3c..16a4655005 100644 --- a/internal/integration/cursor_test.go +++ b/internal/integration/cursor_test.go @@ -102,7 +102,7 @@ func TestCursor_TryNext(t *testing.T) { // insert a document because a tailable cursor will only have a non-zero ID if the initial Find matches // at least one document _, err := mt.Coll.InsertOne(context.Background(), bson.D{{"x", 1}}) - assert.Nil(mt, err, "InsertOne error: %v", err) + require.NoError(mt, err, "InsertOne error: %v", err) cursor, err := mt.Coll.Find(context.Background(), bson.D{}, options.Find().SetCursorType(options.Tailable)) assert.Nil(mt, err, "Find error: %v", err) From a11cc89ce7c4d0fe5afbb25284a52abf58246451 Mon Sep 17 00:00:00 2001 From: Matt Dale <9760375+matthewdale@users.noreply.github.com> Date: Fri, 29 Aug 2025 14:58:49 -0700 Subject: [PATCH 04/12] Apply suggestion from @prestonvasquez Co-authored-by: Preston Vasquez --- internal/integration/cursor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/integration/cursor_test.go b/internal/integration/cursor_test.go index 16a4655005..a1491b6de9 100644 --- a/internal/integration/cursor_test.go +++ b/internal/integration/cursor_test.go @@ -105,7 +105,7 @@ func TestCursor_TryNext(t *testing.T) { require.NoError(mt, err, "InsertOne error: %v", err) cursor, err := mt.Coll.Find(context.Background(), bson.D{}, options.Find().SetCursorType(options.Tailable)) - assert.Nil(mt, err, "Find error: %v", err) + require.NoError(mt, err, "Find error: %v", err) defer cursor.Close(context.Background()) // first call to TryNext should return 1 document From 4e8c1b693705a604f84be6bb92a3f8b0e88c8f8a Mon Sep 17 00:00:00 2001 From: Matt Dale <9760375+matthewdale@users.noreply.github.com> Date: Fri, 29 Aug 2025 14:58:56 -0700 Subject: [PATCH 05/12] Apply suggestion from @prestonvasquez Co-authored-by: Preston Vasquez --- internal/integration/cursor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/integration/cursor_test.go b/internal/integration/cursor_test.go index a1491b6de9..7c6f7491c8 100644 --- a/internal/integration/cursor_test.go +++ b/internal/integration/cursor_test.go @@ -119,7 +119,7 @@ func TestCursor_TryNext(t *testing.T) { findRes := mtest.CreateCursorResponse(50, "foo.bar", mtest.FirstBatch) mt.AddMockResponses(findRes) cursor, err := mt.Coll.Find(context.Background(), bson.D{}) - assert.Nil(mt, err, "Find error: %v", err) + require.NoError(mt, err, "Find error: %v", err) defer cursor.Close(context.Background()) tryNextGetmoreError(mt, cursor) }) From dc52a423b4fa7b9f88f66ec734a8a932ddbb7b66 Mon Sep 17 00:00:00 2001 From: Matt Dale <9760375+matthewdale@users.noreply.github.com> Date: Fri, 29 Aug 2025 14:59:03 -0700 Subject: [PATCH 06/12] Apply suggestion from @prestonvasquez Co-authored-by: Preston Vasquez --- internal/integration/cursor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/integration/cursor_test.go b/internal/integration/cursor_test.go index 7c6f7491c8..82efaa45f6 100644 --- a/internal/integration/cursor_test.go +++ b/internal/integration/cursor_test.go @@ -148,7 +148,7 @@ func TestCursor_RemainingBatchLength(t *testing.T) { SetCursorType(options.TailableAwait). SetMaxAwaitTime(100 * time.Millisecond) cursor, err := mt.Coll.Find(context.Background(), bson.D{}, findOpts) - assert.Nil(mt, err, "Find error: %v", err) + require.NoError(mt, err, "Find error: %v", err) defer cursor.Close(context.Background()) mt.ClearEvents() From b61f4b146298689c63d50454ac81563c1865c427 Mon Sep 17 00:00:00 2001 From: Matt Dale <9760375+matthewdale@users.noreply.github.com> Date: Fri, 29 Aug 2025 14:59:13 -0700 Subject: [PATCH 07/12] Apply suggestion from @prestonvasquez Co-authored-by: Preston Vasquez --- internal/integration/cursor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/integration/cursor_test.go b/internal/integration/cursor_test.go index 82efaa45f6..a8a15a14af 100644 --- a/internal/integration/cursor_test.go +++ b/internal/integration/cursor_test.go @@ -237,7 +237,7 @@ func TestCursor_All(t *testing.T) { var docs []bson.D err = cursor.All(context.Background(), &docs) - assert.NotNil(mt, err, "expected change stream error, got nil") + require.Error(mt, err, "expected change stream error, got nil") // make sure that a mongo.CommandError is returned instead of a driver.Error mongoErr, ok := err.(mongo.CommandError) From 0db413ef250409989d431ab1b9dcb4ea4c564509 Mon Sep 17 00:00:00 2001 From: Matt Dale <9760375+matthewdale@users.noreply.github.com> Date: Fri, 29 Aug 2025 14:59:22 -0700 Subject: [PATCH 08/12] Apply suggestion from @prestonvasquez Co-authored-by: Preston Vasquez --- internal/integration/cursor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/integration/cursor_test.go b/internal/integration/cursor_test.go index a8a15a14af..d4f0954c5f 100644 --- a/internal/integration/cursor_test.go +++ b/internal/integration/cursor_test.go @@ -251,7 +251,7 @@ func TestCursor_All(t *testing.T) { // Find with batchSize 2 so All will run getMore for next 3 docs and error. cur, err := mt.Coll.Find(context.Background(), bson.D{}, options.Find().SetBatchSize(2)) - assert.Nil(mt, err, "Find error: %v", err) + require.NoError(mt, err, "Find error: %v", err) // Create a context and immediately cancel it. canceledCtx, cancel := context.WithCancel(context.Background()) From 49b641fb9a68d03c6ee57806a0dca6a6b34eabc8 Mon Sep 17 00:00:00 2001 From: Matt Dale <9760375+matthewdale@users.noreply.github.com> Date: Fri, 29 Aug 2025 14:59:31 -0700 Subject: [PATCH 09/12] Apply suggestion from @prestonvasquez Co-authored-by: Preston Vasquez --- internal/integration/cursor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/integration/cursor_test.go b/internal/integration/cursor_test.go index d4f0954c5f..e27873f40b 100644 --- a/internal/integration/cursor_test.go +++ b/internal/integration/cursor_test.go @@ -263,7 +263,7 @@ func TestCursor_All(t *testing.T) { // Call All with the canceled context and expect context.Canceled. var docs []bson.D err = cur.All(canceledCtx, &docs) - assert.NotNil(mt, err, "expected error for All, got nil") + require.Error(mt, err, "expected error for All, got nil") assert.True(mt, errors.Is(err, context.Canceled), "expected context.Canceled error, got %v", err) From 95de52d49aa71f4a2909cf41a6700975404dc69e Mon Sep 17 00:00:00 2001 From: Matt Dale <9760375+matthewdale@users.noreply.github.com> Date: Fri, 29 Aug 2025 14:59:40 -0700 Subject: [PATCH 10/12] Apply suggestion from @prestonvasquez Co-authored-by: Preston Vasquez --- internal/integration/cursor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/integration/cursor_test.go b/internal/integration/cursor_test.go index e27873f40b..796b8d577f 100644 --- a/internal/integration/cursor_test.go +++ b/internal/integration/cursor_test.go @@ -307,7 +307,7 @@ func TestCursor_Close(t *testing.T) { }) initCollection(mt, mt.Coll) cursor, err := mt.Coll.Find(context.Background(), bson.D{}, options.Find().SetBatchSize(2)) - assert.Nil(mt, err, "Find error: %v", err) + require.NoError(mt, err, "Find error: %v", err) err = cursor.Close(context.Background()) assert.NotNil(mt, err, "expected change stream error, got nil") From e9f54b349b4bc07cbdebb6fd4d56c2728aa8d6bd Mon Sep 17 00:00:00 2001 From: Matt Dale <9760375+matthewdale@users.noreply.github.com> Date: Fri, 29 Aug 2025 14:59:48 -0700 Subject: [PATCH 11/12] Apply suggestion from @prestonvasquez Co-authored-by: Preston Vasquez --- internal/integration/cursor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/integration/cursor_test.go b/internal/integration/cursor_test.go index 796b8d577f..cfa322f126 100644 --- a/internal/integration/cursor_test.go +++ b/internal/integration/cursor_test.go @@ -310,7 +310,7 @@ func TestCursor_Close(t *testing.T) { require.NoError(mt, err, "Find error: %v", err) err = cursor.Close(context.Background()) - assert.NotNil(mt, err, "expected change stream error, got nil") + require.Error(mt, err, "expected change stream error, got nil") // make sure that a mongo.CommandError is returned instead of a driver.Error mongoErr, ok := err.(mongo.CommandError) From bd7c1255e5a743c1e96e76dbeb569a0575f6982b Mon Sep 17 00:00:00 2001 From: Matt Dale <9760375+matthewdale@users.noreply.github.com> Date: Fri, 29 Aug 2025 15:03:49 -0700 Subject: [PATCH 12/12] Remove assertCursorBatchLength. --- internal/integration/cursor_test.go | 35 ++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/internal/integration/cursor_test.go b/internal/integration/cursor_test.go index cfa322f126..092bcc48a5 100644 --- a/internal/integration/cursor_test.go +++ b/internal/integration/cursor_test.go @@ -155,7 +155,10 @@ func TestCursor_RemainingBatchLength(t *testing.T) { // The initial batch length should be equal to the batchSize. Do batchSize Next calls to exhaust the current // batch and assert that no getMore was done. - assertCursorBatchLength(mt, cursor, batchSize) + assert.Equal(mt, + batchSize, + cursor.RemainingBatchLength(), + "expected remaining batch length to match") for i := 0; i < batchSize; i++ { prevLength := cursor.RemainingBatchLength() if !cursor.Next(context.Background()) { @@ -163,7 +166,10 @@ func TestCursor_RemainingBatchLength(t *testing.T) { } // Each successful Next call should decrement batch length by 1. - assertCursorBatchLength(mt, cursor, prevLength-1) + assert.Equal(mt, + prevLength-1, + cursor.RemainingBatchLength(), + "expected remaining batch length to match") } evt := mt.GetStartedEvent() assert.Nil(mt, evt, "expected no events, got %v", evt) @@ -171,14 +177,20 @@ func TestCursor_RemainingBatchLength(t *testing.T) { // The batch is exhausted, so the batch length should be 0. Do one Next call, which should do a getMore and // fetch batchSize more documents. The batch length after the call should be (batchSize-1) because Next consumes // one document. - assertCursorBatchLength(mt, cursor, 0) + assert.Equal(mt, + 0, + cursor.RemainingBatchLength(), + "expected remaining batch length to match") assert.True(mt, cursor.Next(context.Background()), "expected Next to return true; cursor err: %v", cursor.Err()) evt = mt.GetStartedEvent() assert.NotNil(mt, evt, "expected CommandStartedEvent, got nil") assert.Equal(mt, "getMore", evt.CommandName, "expected command %q, got %q", "getMore", evt.CommandName) - assertCursorBatchLength(mt, cursor, batchSize-1) + assert.Equal(mt, + batchSize-1, + cursor.RemainingBatchLength(), + "expected remaining batch length to match") }) mt.RunOpts("first batch is empty", mtest.NewOptions().ClientType(mtest.Mock), func(mt *mtest.T) { // Test that the cursor reports the correct value for RemainingBatchLength if the first batch is empty. @@ -209,10 +221,16 @@ func TestCursor_RemainingBatchLength(t *testing.T) { } assert.Nil(mt, cursor.Err(), "cursor error: %v", err) - assertCursorBatchLength(mt, cursor, 0) + assert.Equal(mt, + 0, + cursor.RemainingBatchLength(), + "expected remaining batch length to match") } // TryNext consumes one document so the remaining batch size should be len(getMoreBatch)-1. - assertCursorBatchLength(mt, cursor, len(getMoreBatch)-1) + assert.Equal(mt, + len(getMoreBatch)-1, + cursor.RemainingBatchLength(), + "expected remaining batch length to match") }) } @@ -617,8 +635,3 @@ func tryNextGetmoreError(mt *mtest.T, cursor tryNextCursor) { assert.Equal(mt, testErr.Name, mongoErr.Name, "expected name %v, got: %v", testErr.Name, mongoErr.Name) assert.Equal(mt, testErr.Labels, mongoErr.Labels, "expected labels %v, got: %v", testErr.Labels, mongoErr.Labels) } - -func assertCursorBatchLength(mt *mtest.T, cursor *mongo.Cursor, expected int) { - batchLen := cursor.RemainingBatchLength() - assert.Equal(mt, expected, batchLen, "expected remaining batch length %d, got %d", expected, batchLen) -}