-
Notifications
You must be signed in to change notification settings - Fork 1.5k
CSOT: Adjust timeouts and increase test coverage #1383
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
Conversation
JAVA-5104
Rename tests for better clarity. JAVA-5104
Use unified terminiogy for timeout messages. JAVA-5104
JAVA-5104
JAVA-5104
JAVA-5104
JAVA-5104
…sue. - Adress static check issues. JAVA-5104
…s instances. JAVA-5104
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.
Some questions
@@ -97,9 +100,6 @@ | |||
{ | |||
"name": "commitTransaction", | |||
"object": "session", | |||
"arguments": { | |||
"timeoutMS": 500 |
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.
Whats the reasoning for this change?
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.
I have reverted this change as we don't have overrides. Thank you for pointing it out!
* Not a prose spec test. However, it is additional test case for better coverage. | ||
*/ | ||
@Test | ||
@DisplayName("Should refresh timeout for commit transaction") |
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.
How does this test check that the timeout is refreshed? Seems to just confirm that the second commit fail with MongoOperationTimeoutException?
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.
I have corrected the DisplayName
to be Should throw timeout exception for subsequent commit transaction
. Good catch, thanks!
@@ -1,5 +1,6 @@ | |||
{ | |||
"description": "timeoutMS behaves correctly for non-tailable cursors", | |||
"comment": "Manually reduced blockTimeMS for tests to pass in serverless", |
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.
Wowzers
@@ -107,7 +105,6 @@ protected boolean isAsync() { | |||
return true; | |||
} | |||
|
|||
@Tag("setsFailPoint") |
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.
👍
- Rename test name.
# Conflicts: # driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTest.java
@@ -742,10 +732,53 @@ public void shouldIgnoreWtimeoutMsOfWriteConcernToInitialAndSubsequentCommitTran | |||
if (command.containsKey("writeConcern")) { | |||
BsonDocument writeConcern = command.getDocument("writeConcern"); | |||
assertFalse(writeConcern.isEmpty()); | |||
assertFalse(writeConcern.containsKey("wTimeoutMS")); | |||
assertFalse(writeConcern.containsKey("wtimeout")); |
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.
👍
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.
LGTM!
# Conflicts: # driver-sync/src/test/functional/com/mongodb/client/AbstractClientSideOperationsTimeoutProseTest.java # driver-sync/src/test/functional/com/mongodb/client/ClientSideOperationTimeoutTest.java
Description
This PR introduces the following changes:
JAVA-5104