Ensure that resumeToken is included on resume attempts #634
Conversation
...functional/com/mongodb/internal/operation/ChangeStreamOperationProseTestSpecification.groovy
Show resolved
Hide resolved
| // check equality in the case where the batch has not been iterated at all | ||
| assertEquals(cursor.getResumeToken(), batchCursor.getPostBatchResumeToken()); | ||
| // resume token should be null before iteration | ||
| assertNull(cursor.getResumeToken()); |
There was a problem hiding this comment.
This test was wrong. Before the bug fix, both the values were null. Afterwards, the postBatchResumeToken is set on the batch cursor but not the high-level cursor.
There was a problem hiding this comment.
Should you add a assertNotNull(batchCursor.getPostBatchResumeToken()) check as well?
| @@ -100,6 +100,7 @@ | |||
| this.decoder = notNull("decoder", decoder); | |||
| if (result != null) { | |||
| this.operationTime = result.getTimestamp(OPERATION_TIME, null); | |||
| this.postBatchResumeToken = getPostBatchResumeTokenFromResponse(result); | |||
There was a problem hiding this comment.
Now it matches AsyncQueryBatchCursor constructor
| return queryBatchCursor.hasNext(); | ||
| try { | ||
| return queryBatchCursor.hasNext(); | ||
| } catch (RuntimeException e) { |
There was a problem hiding this comment.
It might also be ok for this to be a finally block, but I wasn't able to convince myself of it.
There was a problem hiding this comment.
I'm interested as to why do it this way? Personally, I'd convert to a finally block as its cleaner and in my head equates to the same thing.
There was a problem hiding this comment.
I convinced myself it's ok for this to be a finally block.
| return queryBatchCursor.hasNext(); | ||
| try { | ||
| return queryBatchCursor.hasNext(); | ||
| } catch (RuntimeException e) { |
There was a problem hiding this comment.
I'm interested as to why do it this way? Personally, I'd convert to a finally block as its cleaner and in my head equates to the same thing.
...functional/com/mongodb/internal/operation/ChangeStreamOperationProseTestSpecification.groovy
Show resolved
Hide resolved
| // check equality in the case where the batch has not been iterated at all | ||
| assertEquals(cursor.getResumeToken(), batchCursor.getPostBatchResumeToken()); | ||
| // resume token should be null before iteration | ||
| assertNull(cursor.getResumeToken()); |
There was a problem hiding this comment.
Should you add a assertNotNull(batchCursor.getPostBatchResumeToken()) check as well?
| @@ -51,8 +50,6 @@ public UnifiedTestValidator(final String fileDescription, final String testDescr | |||
|
|
|||
| @Before | |||
| public void setUp() { | |||
| // TODO: remove after https://jira.mongodb.org/browse/JAVA-3871 is fixed | |||
| assumeTrue(!(fileDescription.equals("poc-change-streams") && testDescription.equals("Test consecutive resume"))); | |||
JAVA-3871