Remove client arg from test helpers (#1372)

* remove `client` args from test helpers

The replies to `isMaster`, `serverStatus`, and `find` on `config.shards` are cached. The `client` argument is (surprisingly) ignored after the first call. Remove the argument to signal that the default client is used.

* cache reply in `get_server_params` for consistency

Author: kevinAlbs

src/mongocxx/test/change_streams.cpp

@@ -95,7 +95,7 @@ TEST_CASE("Change stream options") {
     instance::current();
     client mongodb_client{uri{}, test_util::add_test_server_api()};
 
-    if (!test_util::is_replica_set(mongodb_client)) {
+    if (!test_util::is_replica_set()) {
         SKIP("change streams require replica set");
     }
 
@@ -117,7 +117,7 @@ TEST_CASE("Spec Prose Tests") {
     instance::current();
     client client{uri{}, test_util::add_test_server_api()};
 
-    if (!test_util::is_replica_set(client)) {
+    if (!test_util::is_replica_set()) {
         SKIP("change streams require replica set");
     }
 
@@ -375,7 +375,7 @@ TEST_CASE("Mock streams and error-handling") {
 TEST_CASE("Create streams.events and assert we can read a single event", "[min36]") {
     instance::current();
     client mongodb_client{uri{}, test_util::add_test_server_api()};
-    if (!test_util::is_replica_set(mongodb_client)) {
+    if (!test_util::is_replica_set()) {
         SKIP("change streams require replica set");
     }
 
@@ -395,7 +395,7 @@ TEST_CASE("Create streams.events and assert we can read a single event", "[min36
 TEST_CASE("Give an invalid pipeline", "[min36]") {
     instance::current();
     client mongodb_client{uri{}, test_util::add_test_server_api()};
-    if (!test_util::is_replica_set(mongodb_client)) {
+    if (!test_util::is_replica_set()) {
         SKIP("change streams require replica set");
     }
 
@@ -425,7 +425,7 @@ TEST_CASE("Documentation Examples", "[min36]") {
     instance::current();
     mongocxx::pool pool{uri{}, options::pool(test_util::add_test_server_api())};
     auto mongodb_client = pool.acquire();
-    if (!test_util::is_replica_set(*mongodb_client)) {
+    if (!test_util::is_replica_set()) {
         SKIP("change streams require replica set");
     }
 
@@ -531,7 +531,7 @@ TEST_CASE("Documentation Examples", "[min36]") {
 TEST_CASE("Watch 2 collections", "[min36]") {
     instance::current();
     client mongodb_client{uri{}, test_util::add_test_server_api()};
-    if (!test_util::is_replica_set(mongodb_client)) {
+    if (!test_util::is_replica_set()) {
         SKIP("change streams require replica set");
     }
 
@@ -578,7 +578,7 @@ TEST_CASE("Watch 2 collections", "[min36]") {
 TEST_CASE("Watch a Collection", "[min36]") {
     instance::current();
     client mongodb_client{uri{}, test_util::add_test_server_api()};
-    if (!test_util::is_replica_set(mongodb_client)) {
+    if (!test_util::is_replica_set()) {
         SKIP("change streams require replica set");
     }
 

src/mongocxx/test/client.cpp

@@ -387,7 +387,7 @@ TEST_CASE("integration tests for client metadata handshake feature") {
 
             found_op = true;
 
-            std::string server_version = test_util::get_server_version(client);
+            std::string server_version = test_util::get_server_version();
 
             REQUIRE(op_view["clientMetadata"]);
             auto metadata = op_view["clientMetadata"].get_document();

src/mongocxx/test/client_helpers.cpp

@@ -55,18 +55,27 @@ using bsoncxx::builder::basic::make_document;
 
 namespace {
 // These frequently used network calls are cached to avoid bottlenecks during tests.
-document::value get_is_master(client const& client) {
-    static auto reply = client["admin"].run_command(make_document(kvp("isMaster", 1)));
+document::value get_is_master() {
+    static auto reply = []() {
+        auto client = mongocxx::client{mongocxx::uri{}, test_util::add_test_server_api()};
+        return client["admin"].run_command(make_document(kvp("isMaster", 1)));
+    }();
     return reply;
 }
 
-document::value get_server_status(client const& client) {
-    static auto status = client["admin"].run_command(make_document(kvp("serverStatus", 1)));
+document::value get_server_status() {
+    static auto status = []() {
+        auto client = mongocxx::client{mongocxx::uri{}, test_util::add_test_server_api()};
+        return client["admin"].run_command(make_document(kvp("serverStatus", 1)));
+    }();
     return status;
 }
 
-bsoncxx::stdx::optional<document::value> get_shards(client const& client) {
-    static auto shards = client["config"]["shards"].find_one({});
+bsoncxx::stdx::optional<document::value> get_shards() {
+    static auto shards = []() {
+        auto client = mongocxx::client{mongocxx::uri{}, test_util::add_test_server_api()};
+        return client["config"]["shards"].find_one({});
+    }();
     return (shards) ? shards.value() : bsoncxx::stdx::optional<document::value>{};
 }
 } // namespace
@@ -221,8 +230,8 @@ std::int32_t compare_versions(std::string version1, std::string version2) {
     return 0;
 }
 
-bool newer_than(client const& client, std::string version) {
-    auto server_version = get_server_version(client);
+bool newer_than(std::string version) {
+    auto server_version = get_server_version();
     return (compare_versions(server_version, version) >= 0);
 }
 
@@ -248,8 +257,8 @@ options::client add_test_server_api(options::client opts) {
     return opts;
 }
 
-std::int32_t get_max_wire_version(client const& client) {
-    auto reply = get_is_master(client);
+std::int32_t get_max_wire_version() {
+    auto reply = get_is_master();
     auto max_wire_version = reply.view()["maxWireVersion"];
     if (!max_wire_version) {
         // If wire version is not available (i.e. server version too old), it is assumed to be
@@ -262,18 +271,23 @@ std::int32_t get_max_wire_version(client const& client) {
     return max_wire_version.get_int32().value;
 }
 
-std::string get_server_version(client const& client) {
-    auto output = get_server_status(client);
+std::string get_server_version() {
+    auto output = get_server_status();
     return bsoncxx::string::to_string(output.view()["version"].get_string().value);
 }
 
-document::value get_server_params(client const& client) {
-    auto reply = client["admin"].run_command(make_document(kvp("getParameter", "*")));
+document::value get_server_params() {
+    // Cache reply.
+    static auto reply = []() {
+        auto client = mongocxx::client{mongocxx::uri{}, test_util::add_test_server_api()};
+        return client["admin"].run_command(make_document(kvp("getParameter", "*")));
+    }();
+
     return reply;
 }
 
-std::string replica_set_name(client const& client) {
-    auto reply = get_is_master(client);
+std::string replica_set_name() {
+    auto reply = get_is_master();
     auto name = reply.view()["setName"];
     if (name) {
         return bsoncxx::string::to_string(name.get_string().value);
@@ -286,8 +300,8 @@ static bool is_replica_set(document::view reply) {
     return static_cast<bool>(reply["setName"]);
 }
 
-bool is_replica_set(client const& client) {
-    return is_replica_set(get_is_master(client));
+bool is_replica_set() {
+    return is_replica_set(get_is_master());
 }
 
 static bool is_sharded_cluster(document::view reply) {
@@ -300,19 +314,19 @@ static bool is_sharded_cluster(document::view reply) {
     return msg.get_string().value == "isdbgrid";
 }
 
-bool is_sharded_cluster(client const& client) {
-    return is_sharded_cluster(get_is_master(client));
+bool is_sharded_cluster() {
+    return is_sharded_cluster(get_is_master());
 }
 
-std::string get_hosts(client const& client) {
-    auto shards = get_shards(client);
+std::string get_hosts() {
+    auto shards = get_shards();
     if (shards)
         return string::to_string(shards->view()["host"].get_string().value);
     return "";
 }
 
-std::string get_topology(client const& client) {
-    auto const reply = get_is_master(client);
+std::string get_topology() {
+    auto const reply = get_is_master();
 
     if (is_replica_set(reply)) {
         return "replicaset";
@@ -528,8 +542,8 @@ void check_outcome_collection(mongocxx::collection* coll, bsoncxx::document::vie
     REQUIRE(begin(actual) == end(actual));
 }
 
-bool server_has_sessions_impl(client const& conn) {
-    auto result = get_is_master(conn);
+bool server_has_sessions_impl() {
+    auto result = get_is_master();
     auto result_view = result.view();
 
     if (result_view["logicalSessionTimeoutMinutes"]) {

src/mongocxx/test/client_helpers.hh

@@ -52,10 +52,10 @@ namespace test_util {
 std::int32_t compare_versions(std::string version1, std::string version2);
 
 //
-// Returns 'true' if the server version for 'client' is at least 'version',
+// Returns 'true' if the server version for the default client is at least 'version',
 // returns 'false' otherwise.
 //
-bool newer_than(client const& client, std::string version);
+bool newer_than(std::string version);
 
 //
 // Converts a hexadecimal string to an string of bytes.
@@ -77,47 +77,47 @@ std::basic_string<std::uint8_t> convert_hex_string_to_bytes(bsoncxx::stdx::strin
 options::client add_test_server_api(options::client opts = {});
 
 //
-// Determines the max wire version associated with the given client, by running the "hello"
+// Determines the max wire version associated with the default client, by running the "hello"
 // command.
 //
 // Throws mongocxx::operation_exception if the operation fails, or the server reply is malformed.
 //
-std::int32_t get_max_wire_version(client const& client);
+std::int32_t get_max_wire_version();
 
 ///
-/// Determines the server version number by running "serverStatus".
+/// Determines the server version number by running "serverStatus" with the default client.
 ///
-std::string get_server_version(client const& client = {uri{}, add_test_server_api()});
+std::string get_server_version();
 
 ///
-/// Determines the setting of all server parameters by running "getParameter, *".
+/// Determines the setting of all server parameters by running "getParameter, *" with the default client.
 ///
-bsoncxx::document::value get_server_params(client const& client = {uri{}, add_test_server_api()});
+bsoncxx::document::value get_server_params();
 
 ///
-/// Get replica set name, or empty string.
+/// Returns the replica set name or an empty string using the default client.
 ///
-std::string replica_set_name(client const& client);
+std::string replica_set_name();
 
 ///
-/// Determines if the server is a replica set member.
+/// Determines if the server is a replica set member using the default client.
 ///
-bool is_replica_set(client const& client = {uri{}, add_test_server_api()});
+bool is_replica_set();
 
 ///
-/// Determines if the server is a sharded cluster member.
+/// Determines if the server is a sharded cluster member using the default client.
 ///
-bool is_sharded_cluster(client const& client = {uri{}, add_test_server_api()});
+bool is_sharded_cluster();
 
 ///
-/// Returns "standalone", "replicaset", or "sharded".
+/// Returns "standalone", "replicaset", or "sharded" using the default client.
 ///
-std::string get_topology(client const& client = {uri{}, add_test_server_api()});
+std::string get_topology();
 
 ///
-/// Returns the "host" field of the config.shards collection.
+/// Returns the "host" field of the config.shards collection using the default client.
 ///
-std::string get_hosts(client const& client = {uri{}, add_test_server_api()});
+std::string get_hosts();
 
 ///
 /// Parses a JSON file at a given path and return it as a BSON document value.
@@ -204,17 +204,14 @@ auto size(Container c) -> decltype(std::distance(std::begin(c), std::end(c))) {
 //
 // Require a topology that supports sessions (a post-3.6 replica set or cluster of them).
 //
-// @param client
-//   A connected client.
+// @return Whether sessions are supported by the default client's topology.
 //
-// @return Whether sessions are supported by the client's topology.
-//
-bool server_has_sessions_impl(client const& conn);
+bool server_has_sessions_impl();
 
-#define SERVER_HAS_SESSIONS_OR_SKIP(conn)                       \
-    if (!mongocxx::test_util::server_has_sessions_impl(conn)) { \
-        SKIP("server does not support session");                \
-    } else                                                      \
+#define SERVER_HAS_SESSIONS_OR_SKIP()                       \
+    if (!mongocxx::test_util::server_has_sessions_impl()) { \
+        SKIP("server does not support session");            \
+    } else                                                  \
         ((void)0)
 
 #if defined(MONGOC_ENABLE_CLIENT_SIDE_ENCRYPTION)

src/mongocxx/test/client_session.cpp

@@ -46,7 +46,7 @@ TEST_CASE("session options", "[session]") {
 
     client c{uri{}, test_util::add_test_server_api()};
 
-    SERVER_HAS_SESSIONS_OR_SKIP(c);
+    SERVER_HAS_SESSIONS_OR_SKIP();
 
     SECTION("default") {
         // Make sure the defaults don't cause a client exception:
@@ -176,7 +176,7 @@ TEST_CASE("session", "[session]") {
 
     client c{uri{}, test_util::add_test_server_api()};
 
-    SERVER_HAS_SESSIONS_OR_SKIP(c);
+    SERVER_HAS_SESSIONS_OR_SKIP();
 
     auto s = c.start_session();
 
@@ -382,7 +382,7 @@ TEST_CASE("lsid", "[session]") {
 
     session_test test;
 
-    SERVER_HAS_SESSIONS_OR_SKIP(test.client);
+    SERVER_HAS_SESSIONS_OR_SKIP();
 
     auto s = test.client.start_session();
     auto db = test.client["lsid"];
@@ -669,7 +669,7 @@ TEST_CASE("lsid", "[session]") {
     }
 
     SECTION("collection::watch") {
-        if (!test_util::is_replica_set(test.client)) {
+        if (!test_util::is_replica_set()) {
             SKIP("watch() requires replica set");
         }
 
@@ -815,15 +815,15 @@ TEST_CASE("with_transaction", "[session]") {
 
     session_test test;
 
-    SERVER_HAS_SESSIONS_OR_SKIP(test.client);
+    SERVER_HAS_SESSIONS_OR_SKIP();
 
     auto session = test.client.start_session();
 
     // The following three tests are prose tests from the with_transaction spec.
     SECTION("prose tests for with_transaction") {
         SECTION("callback raises a custom error") {
             // Multi-document transactions require server 4.2+.
-            if (compare_versions(get_server_version(test.client), "4.2") < 0) {
+            if (compare_versions(get_server_version(), "4.2") < 0) {
                 SKIP("MongoDB server 4.2 or newer required");
             }
 
@@ -857,7 +857,7 @@ TEST_CASE("unacknowledged write in session", "[session]") {
 
     session_test test;
 
-    SERVER_HAS_SESSIONS_OR_SKIP(test.client);
+    SERVER_HAS_SESSIONS_OR_SKIP();
 
     auto s = test.client.start_session();
     auto db = test.client["lsid"];