[JAVA-4173] Replace use sleep in SyncMongoCursor test class Created: 20/May/21 Updated: 01/Jun/21 Resolved: 01/Jun/21 |
|
| Status: | Closed |
| Project: | Java Driver |
| Component/s: | Test Coverage |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Jeffrey Yemin | Assignee: | Jeffrey Yemin |
| Resolution: | Won't Fix | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Description |
|
Some of the new load balancer unified tests assert on expected events which occur when creating and closing a cursor. But the SyncMongoCursor test class is, necessarily, built on top of a Reactive Streams Publisher, which make it difficult to determine when:
To work around these problems, short sleep}}s were added to both the {{SyncMongoCursor constructor and close method. In limited usage, these sleeps are effective, but ultimately they are racy and some non-zero percentage of time will cause test failures (the percentage can be decreased by increasing the sleep time, but that will slow down test runs). We should consider alternative approaches that are both less expensive in the happy path and generated clearer test failures in the sad path. |
| Comments |
| Comment by Jeffrey Yemin [ 01/Jun/21 ] |
|
Decided to close as won't fix. We can re-open if the sleep approach proves unreliable. |
| Comment by Valentin Kavalenka [ 25/May/21 ] |
|
I see. Thank you for the explanation. Indeed, the current approach seems to be the lesser of evils. |
| Comment by Jeffrey Yemin [ 24/May/21 ] |
|
I tried to implement this, and ran into what seemed to be a road block. Consider this test. It creates a find cursor, and then tries an insert, asserting that the insert failed because of a connection wait queue timeout. In order to make that work, the test runner either has to wait for a little bit after creating the find cursor, or before executing the insert. It makes more sense to do the former than the latter, because it could be almost anything that comes after, not just an insert, that is relying on the cursor to have been established. So we could put a sleep in the createFindCursor handler, but that would also apply to the sync driver. In the end, it seems better to leave the sleep where it is, in the test code that is adapting the reactive driver to the sync driver API. |