Skip to content

Conversation

matthewdale
Copy link
Collaborator

Summary

  • Replace most calls of assert.Nil for error checking with require.NoError, so the test stops as soon as an unexpected error is returned.

Background & Motivation

There's currently a panic while inspecting command monitoring results after an unexpected error happens:

        --- FAIL: TestCollection/insert_many/large_document_batches (2.17s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x28 pc=0x7ff7204f3fa8]
goroutine 650771 [running]:
testing.tRunner.func1.2({0x7ff720641160, 0x7ff720597de0})
	C:/golang/go1.23/src/testing/testing.go:1632 +0x225
testing.tRunner.func1()
	C:/golang/go1.23/src/testing/testing.go:1635 +0x359
panic({0x7ff720641160?, 0x7ff720597de0?})
	C:/golang/go1.23/src/runtime/panic.go:791 +0x132
go.mongodb.org/mongo-driver/v2/internal/integration.TestCollection.func2.3(0xc002a9e000)
	go.mongodb.org/mongo-driver/internal/integration/collection_test.go:178 +0x3e8

@Copilot Copilot AI review requested due to automatic review settings August 19, 2025 23:57
@matthewdale matthewdale requested a review from a team as a code owner August 19, 2025 23:57
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a panic issue in TestCollection by replacing assert.Nil calls with require.NoError for error checking. The change ensures tests stop immediately when an unexpected error occurs, preventing subsequent code from attempting to access nil pointers and causing runtime panics.

  • Replaces ~100 instances of assert.Nil with require.NoError for MongoDB operation error handling
  • Maintains existing error message formatting for better debugging
  • Prevents cascading failures that lead to nil pointer dereferences when accessing command monitoring results

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

matthewdale and others added 2 commits August 19, 2025 17:00
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@matthewdale matthewdale added review-priority-low Low Priority PR for Review: within 3 business days ignore-for-release labels Aug 20, 2025
Copy link
Contributor

🧪 Performance Results

Commit SHA: a9af464

The following benchmark tests for version 68a51018f463160007de09c2 had statistically significant changes (i.e., |z-score| > 1.96):

Benchmark Measurement % Change Patch Value Stable Region H-Score Z-Score
BenchmarkSingleRunCommand ops_per_second_min -52.4547 737.6807 Avg: 1551.5312
Med: 1588.5641
Stdev: 236.7039
0.8693 -3.4383
BenchmarkSmallDocInsertOne ops_per_second_min -44.3901 854.5022 Avg: 1536.6004
Med: 1578.9501
Stdev: 238.7293
0.8224 -2.8572
BenchmarkMultiFindMany total_time_seconds 19.3631 1.2770 Avg: 1.0699
Med: 1.0317
Stdev: 0.0972
0.7901 2.1307
BenchmarkMultiFindMany ops_per_second_med -2.5533 3105590.0621 Avg: 3186963.7136
Med: 3184713.3758
Stdev: 33230.6320
0.7789 -2.4488
BenchmarkLargeDocInsertOne allocated_bytes_per_op -0.2670 5644.0000 Avg: 5659.1111
Med: 5659.0000
Stdev: 6.1531
0.8056 -2.4558
BenchmarkBSONFlatDocumentEncoding allocated_bytes_per_op -0.0934 6266.0000 Avg: 6271.8556
Med: 6272.0000
Stdev: 2.9777
0.7252 -1.9665
BenchmarkBSONDeepDocumentDecoding allocated_bytes_per_op -0.0599 15094.0000 Avg: 15103.0400
Med: 15104.0000
Stdev: 2.6217
0.9023 -3.4481

For a comprehensive view of all microbenchmark results for this PR's commit, please check out the Evergreen perf task for this patch.

Copy link
Contributor

API Change Report

No changes found!

@matthewdale matthewdale merged commit b7fee90 into mongodb:master Aug 20, 2025
36 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug ignore-for-release review-priority-low Low Priority PR for Review: within 3 business days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants