Skip to content

Conversation

matthewdale
Copy link
Collaborator

Summary

Use require.NoError instead of assert.Nil when checking errors in the GridFS integration tests to prevent the test from continuing after an unexpected error.

Background & Motivation

The GridFS tests in the "internal/integration" package will continue after an error and possibly reference uninitialized values, leading to a panic like:

=== RUN   TestGridFS/skipping_download/read_all,_skip_beyond
    gridfs_test.go:77: 
        	Error Trace:	go.mongodb.org\mongo-driver\internal\integration\gridfs_test.go:77
        	            				go.mongodb.org\mongo-driver\internal\integration\mongotest.go:232
        	Error:      	Expected nil, but got: topology.ConnectionError{ConnectionID:"", Wrapped:(*net.OpError)(0xc02b0c8960), init:true, message:"failed to connect to localhost:27017"}
        	Test:       	TestGridFS/skipping_download/read_all,_skip_beyond
        	Messages:   	OpenUploadStream error: connection() error occurred during connection handshake: failed to connect to localhost:27017: dial tcp [::1]:27017: connectex: Only one usage of each socket address (protocol/network address/port) is normally permitted.
        --- FAIL: TestGridFS/skipping_download/read_all,_skip_beyond (0.09s)
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=0x10 pc=0x7ff6d24487f4]
goroutine 419948 [running]:
...
go.mongodb.org/mongo-driver/v2/internal/integration.TestGridFS.func1.1(0xc02ad641c0)
	go.mongodb.org/mongo-driver/internal/integration/gridfs_test.go:79 +0x454
go.mongodb.org/mongo-driver/v2/internal/integration/mtest.(*T).Run.(*T).RunOpts.func1(0xc02b5604e0?)

@Copilot Copilot AI review requested due to automatic review settings August 12, 2025 16:58
@matthewdale matthewdale requested a review from a team as a code owner August 12, 2025 16:58
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 replaces assert.Nil calls with require.NoError when checking for errors in the GridFS integration tests to ensure tests fail immediately on unexpected errors rather than continuing execution and potentially panicking on nil pointer dereferences.

  • Changes error checking assertions from assert.Nil to require.NoError throughout GridFS tests
  • Adds import for the require package
  • Updates one assertion from assert.NotNil to assert.Error for consistency with error checking patterns

Copy link
Contributor

API Change Report

No changes found!

Copy link
Contributor

mongodb-drivers-pr-bot bot commented Aug 12, 2025

🧪 Performance Results

Commit SHA: 085f31f

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

Benchmark Measurement % Change Patch Value Stable Region H-Score Z-Score
BenchmarkMultiFindMany total_mem_allocs 25.8094 459361.0000 Avg: 365124.5000
Med: 364944.5000
Stdev: 12416.6057
0.9371 7.5896
BenchmarkMultiFindMany total_bytes_allocated 25.7991 746845464.0000 Avg: 593681280.0000
Med: 593390168.0000
Stdev: 19635635.7607
0.9388 7.8003
BenchmarkMultiFindMany total_time_seconds 21.5169 1.2717 Avg: 1.0465
Med: 1.0288
Stdev: 0.0780
0.8649 2.8857
BenchmarkSingleFindOneByID total_mem_allocs -15.3834 1407404.0000 Avg: 1663270.8324
Med: 1671894.0000
Stdev: 67901.0461
0.8501 -3.7682
BenchmarkSingleFindOneByID total_bytes_allocated -14.8574 87609184.0000 Avg: 102897048.6550
Med: 103439320.0000
Stdev: 4124822.9042
0.8477 -3.7063
BenchmarkBSONFullDocumentDecoding ops_per_second_min -14.4159 1557.9552 Avg: 1820.3795
Med: 1861.3672
Stdev: 122.7244
0.7580 -2.1383
BenchmarkLargeDocInsertOne ops_per_second_med -13.6832 4929.3136 Avg: 5710.7221
Med: 5715.6738
Stdev: 151.1343
0.8970 -5.1703
BenchmarkMultiInsertSmallDocument total_time_seconds 13.4419 1.2027 Avg: 1.0602
Med: 1.0475
Stdev: 0.0431
0.8498 3.3100
BenchmarkMultiInsertLargeDocument total_mem_allocs -12.1221 1575.0000 Avg: 1792.2593
Med: 1841.0000
Stdev: 109.2413
0.7571 -1.9888
BenchmarkSingleRunCommand total_bytes_allocated -10.4408 88668728.0000 Avg: 99005724.6118
Med: 99683088.0000
Stdev: 4938260.0207
0.7695 -2.0932
BenchmarkSingleFindOneByID total_time_seconds -10.4059 1.0295 Avg: 1.1491
Med: 1.1560
Stdev: 0.0489
0.7734 -2.4451
BenchmarkSingleRunCommand total_mem_allocs -10.0578 960917.0000 Avg: 1068372.2706
Med: 1077155.0000
Stdev: 52893.7285
0.7626 -2.0315
BenchmarkSingleRunCommand ops_per_second_max -9.5228 7722.7233 Avg: 8535.5455
Med: 8565.9708
Stdev: 189.4695
0.8811 -4.2900
BenchmarkMultiInsertSmallDocument total_bytes_allocated 6.1395 473492088.0000 Avg: 446103386.5324
Med: 445522384.0000
Stdev: 13768469.1127
0.7466 1.9892
BenchmarkSingleFindOneByID ops_per_second_med -6.0959 3867.7538 Avg: 4118.8355
Med: 4140.2553
Stdev: 114.7683
0.7665 -2.1877
BenchmarkSmallDocInsertOne total_time_seconds -6.0142 1.1128 Avg: 1.1840
Med: 1.1850
Stdev: 0.0218
0.8330 -3.2660
BenchmarkLargeDocInsertOne total_time_seconds 5.1477 1.2430 Avg: 1.1822
Med: 1.1821
Stdev: 0.0238
0.7822 2.5604
BenchmarkMultiInsertSmallDocument ns_per_op 4.7748 6904.0000 Avg: 6589.3681
Med: 6581.5000
Stdev: 151.4937
0.7403 2.0769
BenchmarkSingleFindOneByID ops_per_second_max -4.4841 4419.8114 Avg: 4627.3037
Med: 4626.5025
Stdev: 105.8524
0.7360 -1.9602
BenchmarkBSONFullDocumentEncoding total_mem_allocs -3.9393 1345485.0000 Avg: 1400661.7412
Med: 1401964.0000
Stdev: 21663.5964
0.7793 -2.5470
BenchmarkBSONFullDocumentEncoding total_bytes_allocated -3.9095 233041504.0000 Avg: 242522981.7412
Med: 242696304.0000
Stdev: 3719834.5002
0.7793 -2.5489
BenchmarkBSONFullDocumentEncoding ns_per_op 3.8451 26615.0000 Avg: 25629.5294
Med: 25538.0000
Stdev: 384.3306
0.7860 2.5641

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

@matthewdale matthewdale added bug review-priority-low Low Priority PR for Review: within 3 business days labels Aug 12, 2025
@matthewdale matthewdale enabled auto-merge (squash) August 12, 2025 19:35
@matthewdale matthewdale changed the title Use require instead of assert for error checking in GriDFS tests. Use require instead of assert for error checking in GridFS tests. Aug 12, 2025
@matthewdale matthewdale merged commit 99707bc into mongodb:master Aug 13, 2025
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug 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