GODRIVER-2589 Clarify *Cursor.All() behavior in comment. (#1804)

Author: qingyang-hu

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())
 	})