-
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-2871 CSOT tests: Expect withTransaction to propagate errors #1553
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,6 +49,8 @@ tests: | |
timeoutMS: 100 | ||
expectError: | ||
isClientError: true | ||
expectError: | ||
isClientError: true | ||
expectEvents: | ||
# The only operation run fails with a client-side error, so there should be no events for the client. | ||
- client: *client | ||
|
@@ -88,9 +90,6 @@ tests: | |
expectEvents: | ||
- client: *client | ||
events: | ||
# Because the second insert expects an error and gets an error, it technically succeeds, so withTransaction | ||
# will try to run commitTransaction. This will fail client-side, though, because the timeout has already | ||
# expired, so no command is sent. | ||
- commandStartedEvent: | ||
commandName: insert | ||
databaseName: *databaseName | ||
|
@@ -103,3 +102,9 @@ tests: | |
command: | ||
insert: *collectionName | ||
maxTimeMS: { $$type: ["int", "long"] } | ||
- commandStartedEvent: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As |
||
commandName: abortTransaction | ||
databaseName: admin | ||
command: | ||
abortTransaction: 1 | ||
maxTimeMS: { $$type: [ "int", "long" ] } |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
In PyMongo (and Go) the "expectError" on the insertOne above causes the test runner to catch, validate, and the ignore the exception so it never reaches the withTransaction block. So this new test would fail. Is it a bug that Java propagates the exception?
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.
Oh I see. This was recently changed in https://jira.mongodb.org/browse/DRIVERS-1709 but wasn't documented in the UTR spec itself. The only reference is in the jira ticket:
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.
Ah I was looking at the old RST version. It is documented in the markdown version: https://github.com/mongodb/specifications/blob/master/source/unified-test-format/unified-test-format.md#withtransaction
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.
To clarify was this change correct?
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.
Yes this change is correct given the new rules for handling errors in the UTR.