GODRIVER-2589 [release/1.17] Clarify *Cursor.All() behavior in comment. (#1839)

Co-authored-by: Qingyang Hu <103950869+qingyang-hu@users.noreply.github.com>

Author: blink1073

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, 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 {

mongo/cursor_test.go

@@ -97,37 +97,69 @@ 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)
+require.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)
+require.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))
+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 {
 expected := bson.D{{"foo", int32(index)}}
 assert.Equal(t, expected, doc, "expected doc %v, got %v", expected, doc)
 }
 })
 
+t.Run("nil slice", func(t *testing.T) {
+cursor, err := newCursor(newTestBatchCursor(0, 0), nil, nil)
+require.NoError(t, err, "newCursor error: %v", err)
+
+var docs []bson.D
+err = cursor.All(context.Background(), &docs)
+require.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)
+require.NoError(t, err, "newCursor error: %v", err)
+
+docs := []bson.D{}
+err = cursor.All(context.Background(), &docs)
+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))
+})
+
+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)
+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))
+})
+
 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)
+require.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))
+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 {
 expected := Document{Foo: int32(index)}
@@ -137,11 +169,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)
+require.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))
+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 {
 expected := bson.D{{"foo", int32(index)}}
@@ -154,31 +186,31 @@ func TestCursor(t *testing.T) {
 
 tbc := newTestBatchCursor(1, 5)
 cursor, err := newCursor(tbc, nil, nil)
-assert.Nil(t, err, "newCursor error: %v", err)
+require.NoError(t, err, "newCursor error: %v", err)
 
 err = cursor.All(context.Background(), &docs)
-assert.Nil(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")
 })
 
 t.Run("does not error given interface as parameter", func(t *testing.T) {
 var docs interface{} = []bson.D{}
 
 cursor, err := newCursor(newTestBatchCursor(1, 5), nil, nil)
-assert.Nil(t, err, "newCursor error: %v", err)
+require.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)))
+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) {
 var docs interface{} = "test"
 
 cursor, err := newCursor(newTestBatchCursor(1, 5), nil, nil)
-assert.Nil(t, err, "newCursor error: %v", err)
+require.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(
@@ -187,15 +219,15 @@ 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"`
 }
 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}}
 
@@ -213,18 +245,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)
+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.Nil(t, err, "Marshal error: %v", err)
+require.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)
+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)
@@ -233,26 +265,26 @@ 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())
+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.Nil(t, err, "Close error: %v", err)
+require.NoError(t, err, "Close error: %v", err)
 })
 
 // Mock an error in a Cursor.
 t.Run("mock Find with error", func(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)
+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())
 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())
 })