Fix getMore with comment specs #1161
Conversation
comandeo
requested review from
jmikola and
benjirewis
and removed request for
a team
There was a problem hiding this comment.
LGTM with added comment to the change stream test. As a sanity check I'd also ask that @ShaneHarvey POC these in Python before we merge.
| @@ -140,7 +140,50 @@ tests: | |||
| - description: "Test that comment is set on getMore" | |||
| runOnRequirements: | |||
| - minServerVersion: "4.4.0" | |||
| topologies: [ single, replicaset ] | |||
There was a problem hiding this comment.
I've noted that single was introduced by ad1ee9b; however, this had no effect as the top-level runOnRequirements specified replica sets and sharded clusters only (962fb5f#diff-ba3c94557f77e83f167fb27ed81eb223e4eb3e6178238f3d93def75f37adb301R3).
| @@ -140,7 +140,50 @@ tests: | |||
| - description: "Test that comment is set on getMore" | |||
| runOnRequirements: | |||
| - minServerVersion: "4.4.0" | |||
| topologies: [ single, replicaset ] | |||
| topologies: [ replicaset ] | |||
There was a problem hiding this comment.
Should you add a comment explaining why sharded clusters are excluded here (e.g. potentially empty getMore responses)?
If we do want to include sharded clusters, I suppose we could utilize ignoreExtraEvents, which was added in ff0c705 for DRIVERS-1713; however, that would then require drivers to implement those test runner changes just to sync these tests.
The best course of action may be to just leave the comment here and open another DRIVERS ticket to come back to these tests and add ignoreExtraEvents to allow sharded clusters. That wouldn't be something you'd need to work on yourself -- reporting the issue and leaving it in the backlog would be sufficient just so we don't lose track of it.
There was a problem hiding this comment.
Good point, thank you. Created a ticked and added corresponding comment to tests.
There was a problem hiding this comment.
I also tested these changes in the Node driver, the tests look good (evergreen run)
mongodb/specifications#1161 updates the specs to only send `comment` on getMore commands if the server version is >= 4.4. Although this functionality was already implemented as a part of this PR, the spec tests were not. This commit brings in the spec affected spec tests.
|
Tested this in the Python driver--evergreen run |
comandeo
force-pushed
the
2243-fix-getmore-comment
branch
from
06749cf
to
f1dc451
Compare
mongodb/specifications#1161 updates the specs to only send `comment` on getMore commands if the server version is >= 4.4. Although this functionality was already implemented as a part of this PR, the spec tests were not. This commit brings in the spec affected spec tests.
https://jira.mongodb.org/browse/DRIVERS-2243 - Fix OperationFailure on MongoDB <4.4.0
https://jira.mongodb.org/browse/DRIVERS-2244 - Test getMore with comment on all topologies