[SERVER-52899] Remove dependency on ScanningReplicaSetMonitor for unit tests Created: 16/Nov/20 Updated: 17/May/21 Resolved: 17/May/21 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Networking |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Matthew Saltz (Inactive) | Assignee: | Randolph Tan |
| Resolution: | Duplicate | Votes: | 0 |
| Labels: | sharding-remove-scanning-rsm, sharding-wfbf-day | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||||||
| Description |
|
Currently dbclient_rs_test and tenant_migration_recipient_service_test use the ScanningReplicaSetMonitor because it supports some mocking capabilities, and the StreamableReplicaSetMonitor does not. Since we're removing production support for ScanningReplicaSetMonitor, we should also remove the need for it in unit tests. Some potential ideas for implementation: If the decided outcome involves the complete removal of the ScanningReplicaSetMonitor, this ticket should either remove all remaining references to the ScanningReplicaSetMonitor from the codebase, or if it makes more sense to do it separately, make a separate ticket to do that. |
| Comments |
| Comment by Lamont Nelson [ 17/Nov/20 ] |
|
I have some ideas for this if you want. I think the current tests rely too much on the internal implementation details for ScanningRSM (I'm thinking of db_client, as I haven't look at the other one that much). There really is only one method that matters in the public api for the RSM, which is getHosts. That method uses the topology manager and server selector to decide what hosts are available to get. With minimal code we could avoid using a mock, and run the production code for the command path in the tests. The tests can build whatever state they need via the TopologyManager onServerDescription, or just install a full pointer to a TopologyDescription to describe the test scenario's topology. There is a fluent api class that facilitates creating server descriptions for testing. This would also require having a 'test mode' flag so the ping and monitoring components don't actually try to connect to real servers. Basically by not actually scheduling any work, such as here. |