-
Notifications
You must be signed in to change notification settings - Fork 239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DRIVERS-1710: Convert retryable reads spec tests to unified test format #1536
Conversation
Drivers may differ in how they query the chunks collection. For instance, PHPLIB queries the chunk's n field with $gte in addition to files_id. Rather than use $$matchAsRoot (schema version 1.13+) to emulate the legacy test behavior of ignoring extra fields, the commandStartedEvent assertion was relaxed and expectResult was introduced to assert that the file is read successfully.
This also renumbers the prose tests
find: *chunks_collection_name | ||
# Avoid checking additional fields since the exact query may | ||
# differ among drivers. expectResult is sufficient to assert | ||
# correctness. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quoting the commit:
Drivers may differ in how they query the chunks collection. For instance, PHPLIB queries the chunk's n field with
$gte
in addition tofiles_id
. Rather than use$$matchAsRoot
(schema version 1.13+) to emulate the legacy test behavior of ignoring extra fields, the commandStartedEvent assertion was relaxed and expectResult was introduced to assert that the file is read successfully.
I know various drivers are behind on schema versions, so sticking with an older version seemed preferable. The chunk query also isn't vital to these tests, and expectResult
with $$matchesHexBytes
is overall an improvement IMO.
|
||
PoolClearedError Retryability Test | ||
================================== | ||
1. PoolClearedError Retryability Test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: prose tests were originally not numbered.
@kkloberdanz: Feel free to remove yourself from the list of reviewers since we've already got two drivers passing the converted tests. |
https://jira.mongodb.org/browse/DRIVERS-1710
Please complete the following before merging:
clusters, and serverless).
POC: mongodb/mongo-php-library#1247