-
Type:
Improvement
-
Resolution: Unresolved
-
Priority:
Major - P3
-
None
-
Affects Version/s: None
-
Component/s: None
-
None
-
Query Execution
-
None
-
None
-
None
-
None
-
None
-
None
-
None
SERVER-130510 removes the some of the infrastructure for testing embedded routers in JavaScript integration tests.
Specifically, SERVER-130510 removes the remaining checks for TestData.testingReplicaSetEndpoint in JavaScript integration tests and hooks. The testingReplicaSetEndpoint flag is known to be never set in any of our test suites, so the removal is safe.
However, during the review of SERVER-130510 there was concern that removing all conditions that require TestData.testingReplicaSetEndpoint to be true can remove test coverage from test hooks.
Consider this hook in file jstests/hooks/run_check_orphans_are_deleted.js:
const topology = DiscoverTopology.findConnectedNodes(conn);if (topology.type == Topology.kShardedCluster) { for (let shardName of Object.keys(topology.shards)) { const shard = topology.shards[shardName]; let shardPrimary; if (shard.type === Topology.kStandalone) { shardPrimary = shard.mongod; } else if (shard.type === Topology.kReplicaSet) { shardPrimary = shard.primary; } else { throw new Error("Unrecognized topology format: " + tojson(topology)); } CheckOrphansAreDeletedHelpers.runCheck( db.getMongo(), newMongoWithRetry(shardPrimary), shardName, ); } } else if ( topology.type == Topology.kReplicaSet && topology.configsvr && TestData.testingReplicaSetEndpoint ) { CheckOrphansAreDeletedHelpers.runCheck( db.getMongo(), newMongoWithRetry(topology.primary), "config", ); } else { throw new Error( "Orphan documents check must be run against a sharded cluster, but got: " + tojson(topology), ); }
The elseif case here requires TestData.testingReplicaSetEndpoint to be truthy, which it provably never is in any of our tests in current master.
In the initial version of SERVER-130510, the elseif case was therefore simply removed. This raised concerns about potentially losing test coverage in this hook for replica sets without the TestData.testingReplicaSetEndpoint flag set. The ask was to keep the call to CheckOrphansAreDeletedHelpers.runCheck(...) for the replica set case, but remove the TestData.testingReplicaSetEndpoint parts from its guarding condition.
The same concern was raised for the hook in file jstests/hooks/run_check_routing_table_consistency.js.
An Evergreen required patch run showed that these branches are not reached even with the TestData.testingReplicaSetEndpoint sub-condition removed. But it is currently still unclear if some test suites outside of the required patch set can reach them. Therefore SERVER-130510 keeps the replica set cases, but adds assert(false); to them to establish if they are actually triggered by any test suite.
If this is the case, the assert will trigger and can be removed. It is then established and can be documented that these cases are still reachable.
If none of the asserts added to the hooks in SERVER-130510 trigger after a while, the replica set cases can be removed entirely from the two hooks.
- is caused by
-
SERVER-130510 Remove testingReplicaSetEndpoint remainders
-
- Closed
-