From e24b5fabf75de76c980e62336521215444c72cd7 Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Wed, 11 Sep 2024 16:18:31 -0400 Subject: [PATCH 1/5] GODRIVER-2589 Clarify `*Cursor.All()` behavior in comment. --- mongo/cursor.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mongo/cursor.go b/mongo/cursor.go index c77d1109f4..ef387988e1 100644 --- a/mongo/cursor.go +++ b/mongo/cursor.go @@ -286,8 +286,9 @@ func (c *Cursor) Close(ctx context.Context) error { } // All iterates the cursor and decodes each document into results. The results parameter must be a pointer to a slice. -// The slice pointed to by results will be completely overwritten. This method will close the cursor after retrieving -// all documents. If the cursor has been iterated, any previously iterated documents will not be included in results. +// The slice pointed to by results will be completely overwritten. A nil slice pointer will not be modified if the cursor +// has been closed or exhausted. This method will close the cursor after retrieving all documents. If the cursor has been +// iterated, any previously iterated documents will not be included in results. // // This method requires driver version >= 1.1.0. func (c *Cursor) All(ctx context.Context, results interface{}) error { From a9774d1423859b2c816ea519b519962f085c1c2e Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Thu, 12 Sep 2024 09:28:20 -0400 Subject: [PATCH 2/5] update tests. --- mongo/cursor_test.go | 74 ++++++++++++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 27 deletions(-) diff --git a/mongo/cursor_test.go b/mongo/cursor_test.go index 3781109019..18ca2673b1 100644 --- a/mongo/cursor_test.go +++ b/mongo/cursor_test.go @@ -102,19 +102,19 @@ func TestCursor(t *testing.T) { t.Run("TestAll", func(t *testing.T) { t.Run("errors if argument is not pointer to slice", func(t *testing.T) { cursor, err := newCursor(newTestBatchCursor(1, 5), nil, nil) - assert.Nil(t, err, "newCursor error: %v", err) + assert.NoError(t, err, "newCursor error: %v", err) err = cursor.All(context.Background(), []bson.D{}) - assert.NotNil(t, err, "expected error, got nil") + assert.Error(t, err, "expected error, got nil") }) t.Run("fills slice with all documents", func(t *testing.T) { cursor, err := newCursor(newTestBatchCursor(1, 5), nil, nil) - assert.Nil(t, err, "newCursor error: %v", err) + assert.NoError(t, err, "newCursor error: %v", err) var docs []bson.D err = cursor.All(context.Background(), &docs) - assert.Nil(t, err, "All error: %v", err) - assert.Equal(t, 5, len(docs), "expected 5 docs, got %v", len(docs)) + assert.NoError(t, err, "All error: %v", err) + assert.Len(t, docs, 5, "expected 5 docs, got %v", len(docs)) for index, doc := range docs { expected := bson.D{{"foo", int32(index)}} @@ -122,17 +122,37 @@ func TestCursor(t *testing.T) { } }) + t.Run("nil slice", func(t *testing.T) { + cursor, err := newCursor(newTestBatchCursor(0, 0), nil, nil) + assert.NoError(t, err, "newCursor error: %v", err) + + var docs []bson.D + err = cursor.All(context.Background(), &docs) + assert.NoError(t, err, "All error: %v", err) + assert.Nil(t, docs, "expected nil docs") + }) + + t.Run("empty slice", func(t *testing.T) { + cursor, err := newCursor(newTestBatchCursor(0, 0), nil, nil) + assert.NoError(t, err, "newCursor error: %v", err) + + docs := []bson.D{{{"foo", "bar"}}, {{"hello", "world"}, {"pi", 3.14159}}} + err = cursor.All(context.Background(), &docs) + assert.NotNil(t, docs, "expected non-nil docs") + assert.Len(t, docs, 0, "expected 0 docs, got %v", len(docs)) + }) + t.Run("decodes each document into slice type", func(t *testing.T) { cursor, err := newCursor(newTestBatchCursor(1, 5), nil, nil) - assert.Nil(t, err, "newCursor error: %v", err) + assert.NoError(t, err, "newCursor error: %v", err) type Document struct { Foo int32 `bson:"foo"` } var docs []Document err = cursor.All(context.Background(), &docs) - assert.Nil(t, err, "All error: %v", err) - assert.Equal(t, 5, len(docs), "expected 5 documents, got %v", len(docs)) + assert.NoError(t, err, "All error: %v", err) + assert.Len(t, docs, 5, "expected 5 documents, got %v", len(docs)) for index, doc := range docs { expected := Document{Foo: int32(index)} @@ -142,11 +162,11 @@ func TestCursor(t *testing.T) { t.Run("multiple batches are included", func(t *testing.T) { cursor, err := newCursor(newTestBatchCursor(2, 5), nil, nil) - assert.Nil(t, err, "newCursor error: %v", err) + assert.NoError(t, err, "newCursor error: %v", err) var docs []bson.D err = cursor.All(context.Background(), &docs) - assert.Nil(t, err, "All error: %v", err) - assert.Equal(t, 10, len(docs), "expected 10 docs, got %v", len(docs)) + assert.NoError(t, err, "All error: %v", err) + assert.Len(t, docs, 10, "expected 10 docs, got %v", len(docs)) for index, doc := range docs { expected := bson.D{{"foo", int32(index)}} @@ -159,10 +179,10 @@ func TestCursor(t *testing.T) { tbc := newTestBatchCursor(1, 5) cursor, err := newCursor(tbc, nil, nil) - assert.Nil(t, err, "newCursor error: %v", err) + assert.NoError(t, err, "newCursor error: %v", err) err = cursor.All(context.Background(), &docs) - assert.Nil(t, err, "All error: %v", err) + assert.NoError(t, err, "All error: %v", err) assert.True(t, tbc.closed, "expected batch cursor to be closed but was not") }) @@ -170,20 +190,20 @@ func TestCursor(t *testing.T) { var docs interface{} = []bson.D{} cursor, err := newCursor(newTestBatchCursor(1, 5), nil, nil) - assert.Nil(t, err, "newCursor error: %v", err) + assert.NoError(t, err, "newCursor error: %v", err) err = cursor.All(context.Background(), &docs) - assert.Nil(t, err, "expected Nil, got error: %v", err) - assert.Equal(t, 5, len(docs.([]bson.D)), "expected 5 documents, got %v", len(docs.([]bson.D))) + assert.NoError(t, err, "All error: %v", err) + assert.Len(t, docs.([]bson.D), 5, "expected 5 documents, got %v", len(docs.([]bson.D))) }) t.Run("errors when not given pointer to slice", func(t *testing.T) { var docs interface{} = "test" cursor, err := newCursor(newTestBatchCursor(1, 5), nil, nil) - assert.Nil(t, err, "newCursor error: %v", err) + assert.NoError(t, err, "newCursor error: %v", err) err = cursor.All(context.Background(), &docs) - assert.NotNil(t, err, "expected error, got: %v", err) + assert.Error(t, err, "expected error, got: %v", err) }) t.Run("with BSONOptions", func(t *testing.T) { cursor, err := newCursor( @@ -192,7 +212,7 @@ func TestCursor(t *testing.T) { UseJSONStructTags: true, }, nil) - require.NoError(t, err, "newCursor error") + require.NoError(t, err, "newCursor error: %v", err) type myDocument struct { A int32 `json:"foo"` @@ -200,7 +220,7 @@ func TestCursor(t *testing.T) { var got []myDocument err = cursor.All(context.Background(), &got) - require.NoError(t, err, "All error") + require.NoError(t, err, "All error: %v", err) want := []myDocument{{A: 0}, {A: 1}, {A: 2}, {A: 3}, {A: 4}} @@ -218,18 +238,18 @@ func TestNewCursorFromDocuments(t *testing.T) { bson.D{{"_id", 2}, {"quux", "quuz"}}, } cur, err := NewCursorFromDocuments(findResult, nil, nil) - assert.Nil(t, err, "NewCursorFromDocuments error: %v", err) + assert.NoError(t, err, "NewCursorFromDocuments error: %v", err) // Assert that decoded documents are as expected. var i int for cur.Next(context.Background()) { docBytes, err := bson.Marshal(findResult[i]) - assert.Nil(t, err, "Marshal error: %v", err) + assert.NoError(t, err, "Marshal error: %v", err) expectedDecoded := bson.Raw(docBytes) var decoded bson.Raw err = cur.Decode(&decoded) - assert.Nil(t, err, "Decode error: %v", err) + assert.NoError(t, err, "Decode error: %v", err) assert.Equal(t, expectedDecoded, decoded, "expected decoded document %v of Cursor to be %v, got %v", i, expectedDecoded, decoded) @@ -238,11 +258,11 @@ func TestNewCursorFromDocuments(t *testing.T) { assert.Equal(t, 3, i, "expected 3 calls to cur.Next, got %v", i) // Check for error on Cursor. - assert.Nil(t, cur.Err(), "Cursor error: %v", cur.Err()) + assert.NoError(t, cur.Err(), "Cursor error: %v", cur.Err()) // Assert that a call to cur.Close will not fail. err = cur.Close(context.Background()) - assert.Nil(t, err, "Close error: %v", err) + assert.NoError(t, err, "Close error: %v", err) }) // Mock an error in a Cursor. @@ -250,14 +270,14 @@ func TestNewCursorFromDocuments(t *testing.T) { mockErr := fmt.Errorf("mock error") findResult := []interface{}{bson.D{{"_id", 0}, {"foo", "bar"}}} cur, err := NewCursorFromDocuments(findResult, mockErr, nil) - assert.Nil(t, err, "NewCursorFromDocuments error: %v", err) + assert.NoError(t, err, "NewCursorFromDocuments error: %v", err) // Assert that a call to Next will return false because of existing error. next := cur.Next(context.Background()) assert.False(t, next, "expected call to Next to return false, got true") // Check for error on Cursor. - assert.NotNil(t, cur.Err(), "expected Cursor error, got nil") + assert.Error(t, cur.Err(), "expected Cursor error, got nil") assert.Equal(t, mockErr, cur.Err(), "expected Cursor error %v, got %v", mockErr, cur.Err()) }) From 3b4d7ab03160ef0f437430530c2cfbf8ecff065c Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Tue, 17 Sep 2024 17:16:41 -0400 Subject: [PATCH 3/5] test updates --- mongo/cursor.go | 4 ++-- mongo/cursor_test.go | 34 +++++++++++++++++++++++----------- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/mongo/cursor.go b/mongo/cursor.go index ef387988e1..1e01e398da 100644 --- a/mongo/cursor.go +++ b/mongo/cursor.go @@ -287,8 +287,8 @@ func (c *Cursor) Close(ctx context.Context) error { // All iterates the cursor and decodes each document into results. The results parameter must be a pointer to a slice. // The slice pointed to by results will be completely overwritten. A nil slice pointer will not be modified if the cursor -// has been closed or exhausted. This method will close the cursor after retrieving all documents. If the cursor has been -// iterated, any previously iterated documents will not be included in results. +// has been closed, exhausted, or is empty. This method will close the cursor after retrieving all documents. If the +// cursor has been iterated, any previously iterated documents will not be included in results. // // This method requires driver version >= 1.1.0. func (c *Cursor) All(ctx context.Context, results interface{}) error { diff --git a/mongo/cursor_test.go b/mongo/cursor_test.go index 18ca2673b1..0d66576707 100644 --- a/mongo/cursor_test.go +++ b/mongo/cursor_test.go @@ -109,7 +109,7 @@ func TestCursor(t *testing.T) { t.Run("fills slice with all documents", func(t *testing.T) { cursor, err := newCursor(newTestBatchCursor(1, 5), nil, nil) - assert.NoError(t, err, "newCursor error: %v", err) + require.NoError(t, err, "newCursor error: %v", err) var docs []bson.D err = cursor.All(context.Background(), &docs) @@ -124,7 +124,7 @@ func TestCursor(t *testing.T) { t.Run("nil slice", func(t *testing.T) { cursor, err := newCursor(newTestBatchCursor(0, 0), nil, nil) - assert.NoError(t, err, "newCursor error: %v", err) + require.NoError(t, err, "newCursor error: %v", err) var docs []bson.D err = cursor.All(context.Background(), &docs) @@ -134,17 +134,29 @@ func TestCursor(t *testing.T) { t.Run("empty slice", func(t *testing.T) { cursor, err := newCursor(newTestBatchCursor(0, 0), nil, nil) - assert.NoError(t, err, "newCursor error: %v", err) + require.NoError(t, err, "newCursor error: %v", err) + + docs := []bson.D{} + err = cursor.All(context.Background(), &docs) + assert.NoError(t, err, "All error: %v", err) + assert.NotNil(t, docs, "expected non-nil docs") + assert.Len(t, docs, 0, "expected 0 docs, got %v", len(docs)) + }) + + t.Run("empty slice overwritten", func(t *testing.T) { + cursor, err := newCursor(newTestBatchCursor(0, 0), nil, nil) + require.NoError(t, err, "newCursor error: %v", err) docs := []bson.D{{{"foo", "bar"}}, {{"hello", "world"}, {"pi", 3.14159}}} err = cursor.All(context.Background(), &docs) + assert.NoError(t, err, "All error: %v", err) assert.NotNil(t, docs, "expected non-nil docs") assert.Len(t, docs, 0, "expected 0 docs, got %v", len(docs)) }) t.Run("decodes each document into slice type", func(t *testing.T) { cursor, err := newCursor(newTestBatchCursor(1, 5), nil, nil) - assert.NoError(t, err, "newCursor error: %v", err) + require.NoError(t, err, "newCursor error: %v", err) type Document struct { Foo int32 `bson:"foo"` @@ -162,7 +174,7 @@ func TestCursor(t *testing.T) { t.Run("multiple batches are included", func(t *testing.T) { cursor, err := newCursor(newTestBatchCursor(2, 5), nil, nil) - assert.NoError(t, err, "newCursor error: %v", err) + require.NoError(t, err, "newCursor error: %v", err) var docs []bson.D err = cursor.All(context.Background(), &docs) assert.NoError(t, err, "All error: %v", err) @@ -179,7 +191,7 @@ func TestCursor(t *testing.T) { tbc := newTestBatchCursor(1, 5) cursor, err := newCursor(tbc, nil, nil) - assert.NoError(t, err, "newCursor error: %v", err) + require.NoError(t, err, "newCursor error: %v", err) err = cursor.All(context.Background(), &docs) assert.NoError(t, err, "All error: %v", err) @@ -190,7 +202,7 @@ func TestCursor(t *testing.T) { var docs interface{} = []bson.D{} cursor, err := newCursor(newTestBatchCursor(1, 5), nil, nil) - assert.NoError(t, err, "newCursor error: %v", err) + require.NoError(t, err, "newCursor error: %v", err) err = cursor.All(context.Background(), &docs) assert.NoError(t, err, "All error: %v", err) @@ -200,7 +212,7 @@ func TestCursor(t *testing.T) { var docs interface{} = "test" cursor, err := newCursor(newTestBatchCursor(1, 5), nil, nil) - assert.NoError(t, err, "newCursor error: %v", err) + require.NoError(t, err, "newCursor error: %v", err) err = cursor.All(context.Background(), &docs) assert.Error(t, err, "expected error, got: %v", err) @@ -238,13 +250,13 @@ func TestNewCursorFromDocuments(t *testing.T) { bson.D{{"_id", 2}, {"quux", "quuz"}}, } cur, err := NewCursorFromDocuments(findResult, nil, nil) - assert.NoError(t, err, "NewCursorFromDocuments error: %v", err) + require.NoError(t, err, "NewCursorFromDocuments error: %v", err) // Assert that decoded documents are as expected. var i int for cur.Next(context.Background()) { docBytes, err := bson.Marshal(findResult[i]) - assert.NoError(t, err, "Marshal error: %v", err) + require.NoError(t, err, "Marshal error: %v", err) expectedDecoded := bson.Raw(docBytes) var decoded bson.Raw @@ -270,7 +282,7 @@ func TestNewCursorFromDocuments(t *testing.T) { mockErr := fmt.Errorf("mock error") findResult := []interface{}{bson.D{{"_id", 0}, {"foo", "bar"}}} cur, err := NewCursorFromDocuments(findResult, mockErr, nil) - assert.NoError(t, err, "NewCursorFromDocuments error: %v", err) + require.NoError(t, err, "NewCursorFromDocuments error: %v", err) // Assert that a call to Next will return false because of existing error. next := cur.Next(context.Background()) From 796367e4fe843b6607956e9a8282b63b7a7bb7b2 Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Fri, 20 Sep 2024 16:57:31 -0400 Subject: [PATCH 4/5] minor change --- mongo/cursor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mongo/cursor_test.go b/mongo/cursor_test.go index 0d66576707..31778d7d58 100644 --- a/mongo/cursor_test.go +++ b/mongo/cursor_test.go @@ -102,7 +102,7 @@ func TestCursor(t *testing.T) { t.Run("TestAll", func(t *testing.T) { t.Run("errors if argument is not pointer to slice", func(t *testing.T) { cursor, err := newCursor(newTestBatchCursor(1, 5), nil, nil) - assert.NoError(t, err, "newCursor error: %v", err) + require.NoError(t, err, "newCursor error: %v", err) err = cursor.All(context.Background(), []bson.D{}) assert.Error(t, err, "expected error, got nil") }) From 1cc21cd64d61b5d428ffe976bd4ac6ac7afb1c15 Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Mon, 23 Sep 2024 18:30:04 -0400 Subject: [PATCH 5/5] update assertion --- mongo/cursor_test.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/mongo/cursor_test.go b/mongo/cursor_test.go index 31778d7d58..23353aacd8 100644 --- a/mongo/cursor_test.go +++ b/mongo/cursor_test.go @@ -113,7 +113,7 @@ func TestCursor(t *testing.T) { var docs []bson.D err = cursor.All(context.Background(), &docs) - assert.NoError(t, err, "All error: %v", err) + require.NoError(t, err, "All error: %v", err) assert.Len(t, docs, 5, "expected 5 docs, got %v", len(docs)) for index, doc := range docs { @@ -128,7 +128,7 @@ func TestCursor(t *testing.T) { var docs []bson.D err = cursor.All(context.Background(), &docs) - assert.NoError(t, err, "All error: %v", err) + require.NoError(t, err, "All error: %v", err) assert.Nil(t, docs, "expected nil docs") }) @@ -138,7 +138,7 @@ func TestCursor(t *testing.T) { docs := []bson.D{} err = cursor.All(context.Background(), &docs) - assert.NoError(t, err, "All error: %v", err) + require.NoError(t, err, "All error: %v", err) assert.NotNil(t, docs, "expected non-nil docs") assert.Len(t, docs, 0, "expected 0 docs, got %v", len(docs)) }) @@ -149,7 +149,7 @@ func TestCursor(t *testing.T) { docs := []bson.D{{{"foo", "bar"}}, {{"hello", "world"}, {"pi", 3.14159}}} err = cursor.All(context.Background(), &docs) - assert.NoError(t, err, "All error: %v", err) + require.NoError(t, err, "All error: %v", err) assert.NotNil(t, docs, "expected non-nil docs") assert.Len(t, docs, 0, "expected 0 docs, got %v", len(docs)) }) @@ -163,7 +163,7 @@ func TestCursor(t *testing.T) { } var docs []Document err = cursor.All(context.Background(), &docs) - assert.NoError(t, err, "All error: %v", err) + require.NoError(t, err, "All error: %v", err) assert.Len(t, docs, 5, "expected 5 documents, got %v", len(docs)) for index, doc := range docs { @@ -177,7 +177,7 @@ func TestCursor(t *testing.T) { require.NoError(t, err, "newCursor error: %v", err) var docs []bson.D err = cursor.All(context.Background(), &docs) - assert.NoError(t, err, "All error: %v", err) + require.NoError(t, err, "All error: %v", err) assert.Len(t, docs, 10, "expected 10 docs, got %v", len(docs)) for index, doc := range docs { @@ -194,7 +194,7 @@ func TestCursor(t *testing.T) { require.NoError(t, err, "newCursor error: %v", err) err = cursor.All(context.Background(), &docs) - assert.NoError(t, err, "All error: %v", err) + require.NoError(t, err, "All error: %v", err) assert.True(t, tbc.closed, "expected batch cursor to be closed but was not") }) @@ -205,7 +205,7 @@ func TestCursor(t *testing.T) { require.NoError(t, err, "newCursor error: %v", err) err = cursor.All(context.Background(), &docs) - assert.NoError(t, err, "All error: %v", err) + require.NoError(t, err, "All error: %v", err) assert.Len(t, docs.([]bson.D), 5, "expected 5 documents, got %v", len(docs.([]bson.D))) }) t.Run("errors when not given pointer to slice", func(t *testing.T) { @@ -261,7 +261,7 @@ func TestNewCursorFromDocuments(t *testing.T) { var decoded bson.Raw err = cur.Decode(&decoded) - assert.NoError(t, err, "Decode error: %v", err) + require.NoError(t, err, "Decode error: %v", err) assert.Equal(t, expectedDecoded, decoded, "expected decoded document %v of Cursor to be %v, got %v", i, expectedDecoded, decoded) @@ -270,11 +270,11 @@ func TestNewCursorFromDocuments(t *testing.T) { assert.Equal(t, 3, i, "expected 3 calls to cur.Next, got %v", i) // Check for error on Cursor. - assert.NoError(t, cur.Err(), "Cursor error: %v", cur.Err()) + require.NoError(t, cur.Err(), "Cursor error: %v", cur.Err()) // Assert that a call to cur.Close will not fail. err = cur.Close(context.Background()) - assert.NoError(t, err, "Close error: %v", err) + require.NoError(t, err, "Close error: %v", err) }) // Mock an error in a Cursor.