Skip to content

Conversation

@patrickfreed
Copy link
Contributor

@patrickfreed patrickfreed commented Aug 22, 2020

SWIFT-958

This fixes a bug where cursors would iterate forever after encountering a non-DecodingError by ensuring next returns nil once after the error. This does not apply to tryNext or toArray, since those are throws. Also, in the case of tryNext, nil doesn't necessarily imply that the cursor is dead.

@patrickfreed patrickfreed requested a review from kmahar August 22, 2020 00:19
var count = 0
for result in cursor {
expect(count).to(beLessThan(2))
if count >= 2 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could not get these tests to halt after failing an expect, so I manually added breaks. I tried setting self.continueAfterFailure = false, but it didn't seem to work.

Copy link
Contributor

Choose a reason for hiding this comment

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

huh, weird 🤔

@codecov-commenter
Copy link

Codecov Report

Merging #521 into master will decrease coverage by 0.00%.
The diff coverage is 67.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #521      +/-   ##
==========================================
- Coverage   77.65%   77.65%   -0.01%     
==========================================
  Files         130      130              
  Lines       13876    13910      +34     
==========================================
+ Hits        10776    10802      +26     
- Misses       3100     3108       +8     
Impacted Files Coverage Δ
Sources/TestsCommon/ServerVersion.swift 93.93% <ø> (ø)
Tests/MongoSwiftSyncTests/MongoCursorTests.swift 91.04% <40.74%> (-7.81%) ⬇️
Sources/TestsCommon/CommonTestUtils.swift 67.31% <85.71%> (+0.81%) ⬆️
Sources/MongoSwift/CursorCommon.swift 95.34% <100.00%> (+0.22%) ⬆️
Tests/MongoSwiftSyncTests/SyncTestUtils.swift 68.31% <100.00%> (+6.18%) ⬆️
Tests/MongoSwiftTests/MongoCursorTests.swift 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9e8cba...19e2927. Read the comment docs.

Copy link
Contributor

@kmahar kmahar left a comment

Choose a reason for hiding this comment

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

lgtm!

var count = 0
for result in cursor {
expect(count).to(beLessThan(2))
if count >= 2 {
Copy link
Contributor

Choose a reason for hiding this comment

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

huh, weird 🤔

@patrickfreed patrickfreed merged commit 366e88c into mongodb:master Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants