From c59c85c6ddedfd3f0f973e477adb90b51d1dae5e Mon Sep 17 00:00:00 2001 From: Kevin Albertson Date: Wed, 19 Mar 2025 17:44:33 +0000 Subject: [PATCH 1/6] 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. --- src/mongocxx/test/client_helpers.cpp | 57 ++++++++++++++++------------ src/mongocxx/test/client_helpers.hh | 47 +++++++++++------------ 2 files changed, 55 insertions(+), 49 deletions(-) diff --git a/src/mongocxx/test/client_helpers.cpp b/src/mongocxx/test/client_helpers.cpp index 81cf9d11db..dcd5e7b7f4 100644 --- a/src/mongocxx/test/client_helpers.cpp +++ b/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()); + 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()); + return client["admin"].run_command(make_document(kvp("serverStatus", 1))); + }(); return status; } -bsoncxx::stdx::optional get_shards(client const& client) { - static auto shards = client["config"]["shards"].find_one({}); +bsoncxx::stdx::optional get_shards() { + static auto shards = []() { + auto client = mongocxx::client(mongocxx::uri()); + return client["config"]["shards"].find_one({}); + }(); return (shards) ? shards.value() : bsoncxx::stdx::optional{}; } } // 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,8 +271,8 @@ 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); } @@ -272,8 +281,8 @@ document::value get_server_params(client const& client) { 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 +295,8 @@ static bool is_replica_set(document::view reply) { return static_cast(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 +309,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 +537,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"]) { diff --git a/src/mongocxx/test/client_helpers.hh b/src/mongocxx/test/client_helpers.hh index 63522a5e6d..df3e417daf 100644 --- a/src/mongocxx/test/client_helpers.hh +++ b/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,17 +77,17 @@ std::basic_string 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, *". @@ -95,29 +95,29 @@ std::string get_server_version(client const& client = {uri{}, add_test_server_ap bsoncxx::document::value get_server_params(client const& client = {uri{}, add_test_server_api()}); /// -/// 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) From 6c867d6a968df2e43ff33fefbb172a022406631b Mon Sep 17 00:00:00 2001 From: Kevin Albertson Date: Wed, 19 Mar 2025 17:44:51 +0000 Subject: [PATCH 2/6] remove `client` arg from calls --- src/mongocxx/test/change_streams.cpp | 14 +++---- src/mongocxx/test/client.cpp | 2 +- src/mongocxx/test/client_session.cpp | 14 +++---- src/mongocxx/test/client_side_encryption.cpp | 40 +++++++++---------- src/mongocxx/test/collection.cpp | 30 +++++++------- src/mongocxx/test/database.cpp | 2 +- src/mongocxx/test/index_view.cpp | 6 +-- src/mongocxx/test/sdam-monitoring.cpp | 5 +-- src/mongocxx/test/spec/gridfs.cpp | 2 +- .../test/spec/unified_tests/runner.cpp | 2 +- src/mongocxx/test/spec/util.cpp | 6 +-- src/mongocxx/test/transactions.cpp | 8 ++-- src/mongocxx/test/versioned_api.cpp | 6 +-- 13 files changed, 68 insertions(+), 69 deletions(-) diff --git a/src/mongocxx/test/change_streams.cpp b/src/mongocxx/test/change_streams.cpp index 154b713f85..5abf7edeee 100644 --- a/src/mongocxx/test/change_streams.cpp +++ b/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"); } @@ -378,7 +378,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"); } @@ -398,7 +398,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"); } @@ -428,7 +428,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"); } @@ -534,7 +534,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"); } @@ -581,7 +581,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"); } diff --git a/src/mongocxx/test/client.cpp b/src/mongocxx/test/client.cpp index 682da526bf..5593229eec 100644 --- a/src/mongocxx/test/client.cpp +++ b/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(); diff --git a/src/mongocxx/test/client_session.cpp b/src/mongocxx/test/client_session.cpp index a9579c2d36..54ed8982b9 100644 --- a/src/mongocxx/test/client_session.cpp +++ b/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,7 +815,7 @@ 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(); @@ -823,7 +823,7 @@ TEST_CASE("with_transaction", "[session]") { 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"]; diff --git a/src/mongocxx/test/client_side_encryption.cpp b/src/mongocxx/test/client_side_encryption.cpp index 3670aa4411..98491a9df8 100644 --- a/src/mongocxx/test/client_side_encryption.cpp +++ b/src/mongocxx/test/client_side_encryption.cpp @@ -342,7 +342,7 @@ TEST_CASE("Datakey and double encryption", "[client_side_encryption]") { mongocxx::client setup_client{uri{}, test_util::add_test_server_api(client_opts)}; - if (test_util::get_max_wire_version(setup_client) < 8) { + if (test_util::get_max_wire_version() < 8) { // Automatic encryption requires wire version 8. SKIP("max wire version is < 8"); } @@ -578,7 +578,7 @@ TEST_CASE("External key vault", "[client_side_encryption]") { test_util::add_test_server_api(), }; - if (test_util::get_max_wire_version(setup_client) < 8) { + if (test_util::get_max_wire_version() < 8) { // Automatic encryption requires wire version 8. SKIP("max wire version is < 8"); } @@ -598,7 +598,7 @@ TEST_CASE("BSON size limits and batch splitting", "[client_side_encryption]") { test_util::add_test_server_api(), }; - if (test_util::get_max_wire_version(client) < 8) { + if (test_util::get_max_wire_version() < 8) { // Automatic encryption requires wire version 8. SKIP("max wire version is < 8"); } @@ -756,7 +756,7 @@ TEST_CASE("Views are prohibited", "[client_side_encryption]") { test_util::add_test_server_api(), }; - if (test_util::get_max_wire_version(client) < 8) { + if (test_util::get_max_wire_version() < 8) { // Automatic encryption requires wire version 8. SKIP("max wire version is < 8"); } @@ -1096,7 +1096,7 @@ TEST_CASE("Corpus", "[client_side_encryption]") { test_util::add_test_server_api(), }; - if (test_util::get_max_wire_version(setup_client) < 8) { + if (test_util::get_max_wire_version() < 8) { // Automatic encryption requires wire version 8. SKIP("max wire version is < 8"); } @@ -1216,7 +1216,7 @@ TEST_CASE("Custom endpoint", "[client_side_encryption]") { test_util::add_test_server_api(), }; - if (test_util::get_max_wire_version(setup_client) < 8) { + if (test_util::get_max_wire_version() < 8) { // Automatic encryption requires wire version 8. SKIP("max wire version is < 8"); } @@ -1565,7 +1565,7 @@ TEST_CASE("Bypass spawning mongocryptd", "[client_side_encryption]") { test_util::add_test_server_api(), }; - if (test_util::get_max_wire_version(setup_client) < 8) { + if (test_util::get_max_wire_version() < 8) { // Automatic encryption requires wire version 8. SKIP("max wire version is < 8"); } @@ -1708,7 +1708,7 @@ TEST_CASE("KMS TLS expired certificate", "[client_side_encryption]") { SKIP("KMS TLS tests disabled (BUILD-14068)"); } - if (test_util::get_max_wire_version(setup_client) < 8) { + if (test_util::get_max_wire_version() < 8) { // Automatic encryption requires wire version 8. SKIP("max wire version is < 8"); } @@ -1764,7 +1764,7 @@ TEST_CASE("KMS TLS wrong host certificate", "[client_side_encryption]") { SKIP("KMS TLS tests disabled (BUILD-14068)"); } - if (test_util::get_max_wire_version(setup_client) < 8) { + if (test_util::get_max_wire_version() < 8) { // Automatic encryption requires wire version 8. SKIP("max wire version is < 8"); } @@ -1873,7 +1873,7 @@ TEST_CASE("KMS TLS Options Tests", "[client_side_encryption][!mayfail]") { SKIP("KMS TLS tests disabled (BUILD-14068)"); } - if (test_util::get_max_wire_version(setup_client) < 8) { + if (test_util::get_max_wire_version() < 8) { // Automatic encryption requires wire version 8. SKIP("max wire version is < 8"); } @@ -2125,11 +2125,11 @@ TEST_CASE("Explicit Encryption", "[client_side_encryption]") { test_util::add_test_server_api(), }; - if (!test_util::newer_than(conn, "7.0")) { + if (!test_util::newer_than("7.0")) { SKIP("MongoDB server 7.0 or newer required"); } - if (test_util::get_topology(conn) == "single") { + if (test_util::get_topology() == "single") { SKIP("must not run against a standalone server"); } @@ -2425,11 +2425,11 @@ TEST_CASE("Create Encrypted Collection", "[client_side_encryption]") { mongocxx::client conn{mongocxx::uri{}, test_util::add_test_server_api()}; - if (!test_util::newer_than(conn, "7.0")) { + if (!test_util::newer_than("7.0")) { SKIP("Explicit Encryption tests require MongoDB server 7.0+."); } - if (test_util::get_topology(conn) == "single") { + if (test_util::get_topology() == "single") { SKIP("Explicit Encryption tests must not run against a standalone."); } @@ -2554,7 +2554,7 @@ TEST_CASE("Unique Index on keyAltNames", "[client_side_encryption]") { CLIENT_SIDE_ENCRYPTION_ENABLED_OR_SKIP(); - if (!test_util::newer_than(uri{}, "4.2")) { + if (!test_util::newer_than("4.2")) { SKIP("requires MongoDB server 4.2+"); } @@ -2692,7 +2692,7 @@ TEST_CASE("Custom Key Material Test", "[client_side_encryption]") { CLIENT_SIDE_ENCRYPTION_ENABLED_OR_SKIP(); - if (!test_util::newer_than(uri{}, "4.2")) { + if (!test_util::newer_than("4.2")) { SKIP("MongoDB server 4.2 or newer required"); } @@ -3037,15 +3037,15 @@ TEST_CASE("Range Explicit Encryption", "[client_side_encryption]") { { auto client = mongocxx::client(mongocxx::uri(), test_util::add_test_server_api()); - if (!test_util::newer_than(client, "8.0")) { + if (!test_util::newer_than("8.0")) { SKIP("MongoDB server 8.0 or newer required"); } - if (test_util::get_topology(client) == "single") { + if (test_util::get_topology() == "single") { SKIP("must not run against a standalone server"); } - is_replica_set = test_util::get_topology(client) == "replicaset"; + is_replica_set = test_util::get_topology() == "replicaset"; } RangeFieldType const field_types[] = { @@ -3442,7 +3442,7 @@ TEST_CASE("Range Explicit Encryption applies defaults", "[client_side_encryption test_util::add_test_server_api(), }; - if (!test_util::newer_than(key_vault_client, "8.0")) { + if (!test_util::newer_than("8.0")) { SKIP("MongoDB server 8.0 or newer required"); } diff --git a/src/mongocxx/test/collection.cpp b/src/mongocxx/test/collection.cpp index 4b5e029f41..a21e5a7985 100644 --- a/src/mongocxx/test/collection.cpp +++ b/src/mongocxx/test/collection.cpp @@ -220,7 +220,7 @@ TEST_CASE("CRUD functionality", "[driver::collection]") { } SECTION("unacknowledged write concern returns disengaged optional", "[collection]") { - if (test_util::get_max_wire_version(mongodb_client) > 13) { + if (test_util::get_max_wire_version() > 13) { SKIP("getLastError removed in SERVER-57390"); } collection coll = db["insert_one_unack_write"]; @@ -347,7 +347,7 @@ TEST_CASE("CRUD functionality", "[driver::collection]") { } SECTION("unacknowledged write concern returns disengaged optional") { - if (test_util::get_max_wire_version(mongodb_client) > 13) { + if (test_util::get_max_wire_version() > 13) { SKIP("getLastError removed in SERVER-57390"); } collection coll = db["insert_many_unack_write"]; @@ -536,7 +536,7 @@ TEST_CASE("CRUD functionality", "[driver::collection]") { } SECTION("update_one can take a pipeline", "[collection]") { - if (!test_util::newer_than(mongodb_client, "4.1.11")) { + if (!test_util::newer_than("4.1.11")) { SKIP("pipeline updates require 4.1.11"); } @@ -606,7 +606,7 @@ TEST_CASE("CRUD functionality", "[driver::collection]") { } SECTION("unacknowledged write concern returns disengaged optional") { - if (test_util::get_max_wire_version(mongodb_client) > 13) { + if (test_util::get_max_wire_version() > 13) { SKIP("getLastError removed in SERVER-57390"); } collection coll = db["update_one_unack_write"]; @@ -715,7 +715,7 @@ TEST_CASE("CRUD functionality", "[driver::collection]") { } SECTION("unacknowledged write concern returns disengaged optional") { - if (test_util::get_max_wire_version(mongodb_client) > 13) { + if (test_util::get_max_wire_version() > 13) { SKIP("getLastError removed in SERVER-57390"); } collection coll = db["update_many_unack_write"]; @@ -884,7 +884,7 @@ TEST_CASE("CRUD functionality", "[driver::collection]") { } SECTION("unacknowledged write concern returns disengaged optional") { - if (test_util::get_max_wire_version(mongodb_client) > 13) { + if (test_util::get_max_wire_version() > 13) { SKIP("getLastError removed in SERVER-57390"); } collection coll = db["replace_one_unack_write"]; @@ -1017,7 +1017,7 @@ TEST_CASE("CRUD functionality", "[driver::collection]") { } SECTION("unacknowledged write concern returns disengaged optional") { - if (test_util::get_max_wire_version(mongodb_client) > 13) { + if (test_util::get_max_wire_version() > 13) { SKIP("getLastError removed in SERVER-57390"); } collection coll = db["delete_one_unack_write"]; @@ -1117,7 +1117,7 @@ TEST_CASE("CRUD functionality", "[driver::collection]") { } SECTION("unacknowledged write concern returns disengaged optional") { - if (test_util::get_max_wire_version(mongodb_client) > 13) { + if (test_util::get_max_wire_version() > 13) { SKIP("getLastError removed in SERVER-57390"); } collection coll = db["delete_many_unack_write"]; @@ -1687,7 +1687,7 @@ TEST_CASE("CRUD functionality", "[driver::collection]") { SECTION("merge") { auto merge_version = "4.1.11"; - auto server_version = test_util::get_server_version(mongodb_client); + auto server_version = test_util::get_server_version(); if (test_util::compare_versions(server_version, merge_version) < 0) { // The server does not support $merge. return; @@ -1975,7 +1975,7 @@ TEST_CASE("CRUD functionality", "[driver::collection]") { } SECTION("unacknowledged write concern returns disengaged optional", "[collection]") { - if (test_util::get_max_wire_version(mongodb_client) > 13) { + if (test_util::get_max_wire_version() > 13) { SKIP("getLastError removed in SERVER-57390"); } collection coll = db["bulk_write_unack_write"]; @@ -2222,7 +2222,7 @@ TEST_CASE("create_index tests", "[collection]") { options::index options{}; options.unique(true); - if (test_util::newer_than(mongodb_client, "4.4")) + if (test_util::newer_than("4.4")) options.hidden(true); options.expire_after(std::chrono::seconds(500)); options.name(index_name); @@ -2239,7 +2239,7 @@ TEST_CASE("create_index tests", "[collection]") { REQUIRE(unique_ele.type() == type::k_bool); REQUIRE(unique_ele.get_bool() == options.unique().value()); - if (test_util::newer_than(mongodb_client, "4.4")) { + if (test_util::newer_than("4.4")) { auto hidden_ele = index["hidden"]; REQUIRE(hidden_ele); REQUIRE(hidden_ele.type() == type::k_bool); @@ -2544,8 +2544,8 @@ TEST_CASE("Ensure that the WriteConcernError 'errInfo' object is propagated", "[ client mongodb_client{uri{}, test_util::add_test_server_api()}; - if (test_util::get_topology(mongodb_client) == "sharded" && - test_util::compare_versions(test_util::get_server_version(mongodb_client), "4.1.0") < 0) { + if (test_util::get_topology() == "sharded" && + test_util::compare_versions(test_util::get_server_version(), "4.1.0") < 0) { SKIP("failCommand on mongos requires 4.1+"); } @@ -2647,7 +2647,7 @@ TEST_CASE("expose writeErrors[].errInfo", "[collection]") { auto mongodb_client = mongocxx::client(uri{}, client_opts); - if (!test_util::newer_than(mongodb_client, "5.0")) { + if (!test_util::newer_than("5.0")) { SKIP("test requires MongoDB server 5.0 or newer"); } diff --git a/src/mongocxx/test/database.cpp b/src/mongocxx/test/database.cpp index 14377f4500..5789a9718c 100644 --- a/src/mongocxx/test/database.cpp +++ b/src/mongocxx/test/database.cpp @@ -334,7 +334,7 @@ TEST_CASE("Database integration tests", "[database]") { }; SECTION("listLocalSessions") { - SERVER_HAS_SESSIONS_OR_SKIP(mongo_client); + SERVER_HAS_SESSIONS_OR_SKIP(); // SERVER-79306: Ensure the database exists for consistent behavior with sharded // clusters. diff --git a/src/mongocxx/test/index_view.cpp b/src/mongocxx/test/index_view.cpp index 1114a6a694..a299c081ab 100644 --- a/src/mongocxx/test/index_view.cpp +++ b/src/mongocxx/test/index_view.cpp @@ -46,7 +46,7 @@ bool test_commands_enabled(client const& conn) { return false; } - auto server_version = test_util::get_server_version(conn); + auto server_version = test_util::get_server_version(); if (test_util::compare_versions(server_version, "3.2") >= 0) { return result_view["enableTestCommands"].get_bool(); @@ -178,7 +178,7 @@ TEST_CASE("create_one", "[index_view]") { } SECTION("commitQuorum option") { - if (test_util::get_topology(mongodb_client) == "single") { + if (test_util::get_topology() == "single") { SKIP("commitQuorum option requires a replica set"); } @@ -193,7 +193,7 @@ TEST_CASE("create_one", "[index_view]") { auto commit_quorum_regex = Catch::Matchers::Matches("(.*)commit( )?quorum(.*)", Catch::CaseSensitive::No); - bool is_supported = test_util::get_max_wire_version(mongodb_client) >= 9; + bool is_supported = test_util::get_max_wire_version() >= 9; CAPTURE(is_supported); SECTION("works with int") { diff --git a/src/mongocxx/test/sdam-monitoring.cpp b/src/mongocxx/test/sdam-monitoring.cpp index f5094600ff..0b01982e6e 100644 --- a/src/mongocxx/test/sdam-monitoring.cpp +++ b/src/mongocxx/test/sdam-monitoring.cpp @@ -49,10 +49,9 @@ TEST_CASE("SDAM Monitoring", "[sdam_monitoring]") { std::string rs_name; uri test_uri; - client discoverer{uri{}, test_util::add_test_server_api()}; - auto topology_type = test_util::get_topology(discoverer); + auto topology_type = test_util::get_topology(); if (topology_type == "replicaset") { - rs_name = test_util::replica_set_name(discoverer); + rs_name = test_util::replica_set_name(); test_uri = uri{"mongodb://localhost/?replicaSet=" + rs_name}; } diff --git a/src/mongocxx/test/spec/gridfs.cpp b/src/mongocxx/test/spec/gridfs.cpp index aacbba2ce3..c6f69ac678 100644 --- a/src/mongocxx/test/spec/gridfs.cpp +++ b/src/mongocxx/test/spec/gridfs.cpp @@ -474,7 +474,7 @@ TEST_CASE("GridFS spec automated tests", "[gridfs_spec]") { // Because the GridFS spec tests use write commands that were only added to MongoDB in version // 2.6, the tests will not run against any server versions older than that. - if (test_util::compare_versions(test_util::get_server_version(client), "2.6") < 0) { + if (test_util::compare_versions(test_util::get_server_version(), "2.6") < 0) { return; } diff --git a/src/mongocxx/test/spec/unified_tests/runner.cpp b/src/mongocxx/test/spec/unified_tests/runner.cpp index 36ead24608..fb9a4b563e 100644 --- a/src/mongocxx/test/spec/unified_tests/runner.cpp +++ b/src/mongocxx/test/spec/unified_tests/runner.cpp @@ -384,7 +384,7 @@ std::string get_hostnames(bsoncxx::document::view object) { static constexpr auto two = "localhost:27017,localhost:27018"; static constexpr auto three = "localhost:27017,localhost:27018,localhost:27019"; - auto const topology = test_util::get_topology(client0); + auto const topology = test_util::get_topology(); if (topology == "single") { return one; // Single mongod. diff --git a/src/mongocxx/test/spec/util.cpp b/src/mongocxx/test/spec/util.cpp index 356b7301d9..0b3ebd9014 100644 --- a/src/mongocxx/test/spec/util.cpp +++ b/src/mongocxx/test/spec/util.cpp @@ -136,9 +136,9 @@ bool check_if_skip_spec_test_impl(client const& client, document::view test, std } } - auto const server_version = test_util::get_server_version(client); + auto const server_version = test_util::get_server_version(); - auto const topology = test_util::get_topology(client); + auto const topology = test_util::get_topology(); if (test["ignore_if_server_version_greater_than"]) { auto const max_server_version = @@ -337,7 +337,7 @@ void set_up_collection( // When testing against a sharded cluster run a `distinct` command on the newly created // collection on all mongoses. - if (test_util::is_sharded_cluster(client)) { + if (test_util::is_sharded_cluster()) { auto s0 = mongocxx::client(uri("mongodb://localhost:27017")); auto s1 = mongocxx::client(uri("mongodb://localhost:27018")); diff --git a/src/mongocxx/test/transactions.cpp b/src/mongocxx/test/transactions.cpp index b0f1df1c17..cfc9bad4b2 100644 --- a/src/mongocxx/test/transactions.cpp +++ b/src/mongocxx/test/transactions.cpp @@ -36,9 +36,9 @@ TEST_CASE("Transaction tests", "[transactions]") { 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("transactions tests require replica set"); - } else if (test_util::get_max_wire_version(mongodb_client) < 7) { + } else if (test_util::get_max_wire_version() < 7) { SKIP("transactions tests require max wire version is >= 7"); } @@ -218,9 +218,9 @@ TEST_CASE("Transactions Documentation Examples", "[transactions]") { 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("transactions tests require replica set"); - } else if (test_util::get_max_wire_version(client) < 7) { + } else if (test_util::get_max_wire_version() < 7) { SKIP("transactions tests require max wire version is >= 7"); } diff --git a/src/mongocxx/test/versioned_api.cpp b/src/mongocxx/test/versioned_api.cpp index 64ec308cea..6a092fab51 100644 --- a/src/mongocxx/test/versioned_api.cpp +++ b/src/mongocxx/test/versioned_api.cpp @@ -32,7 +32,7 @@ using namespace mongocxx; static bool has_api_version_1( mongocxx::client const& client = mongocxx::client(uri(), test_util::add_test_server_api())) { // API Version 1 was introduced in 5.0. - return test_util::get_max_wire_version(client) >= 13; + return test_util::get_max_wire_version() >= 13; } static bool has_api_version_1_with_count( @@ -41,7 +41,7 @@ static bool has_api_version_1_with_count( return false; } - auto const version = test_util::get_server_version(client); + auto const version = test_util::get_server_version(); // BACKPORT-12171: count command was backported to 5.0.9. if (test_util::compare_versions(version, "5.0") == 0 && test_util::compare_versions(version, "5.0.9") >= 0) { @@ -54,7 +54,7 @@ static bool has_api_version_1_with_count( } // SERVER-63850: count command was added in 6.0. - return test_util::newer_than(client, "6.0"); + return test_util::newer_than("6.0"); } // We'll format many of these examples by hand From 920fe06de27656995802fac2d74b4da9e73112b3 Mon Sep 17 00:00:00 2001 From: Kevin Albertson Date: Wed, 19 Mar 2025 17:46:36 +0000 Subject: [PATCH 3/6] cache reply in `get_server_params` for consistency --- src/mongocxx/test/client_helpers.cpp | 9 +++++++-- src/mongocxx/test/client_helpers.hh | 4 ++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/mongocxx/test/client_helpers.cpp b/src/mongocxx/test/client_helpers.cpp index dcd5e7b7f4..5a48a7eeef 100644 --- a/src/mongocxx/test/client_helpers.cpp +++ b/src/mongocxx/test/client_helpers.cpp @@ -276,8 +276,13 @@ std::string get_server_version() { 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()); + return client["admin"].run_command(make_document(kvp("getParameter", "*"))); + }(); + return reply; } diff --git a/src/mongocxx/test/client_helpers.hh b/src/mongocxx/test/client_helpers.hh index df3e417daf..96a550c84c 100644 --- a/src/mongocxx/test/client_helpers.hh +++ b/src/mongocxx/test/client_helpers.hh @@ -90,9 +90,9 @@ std::int32_t get_max_wire_version(); 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(); /// /// Returns the replica set name or an empty string using the default client. From c3500c9509cdff12bae8b3a7a6303757b4f46e5a Mon Sep 17 00:00:00 2001 From: Kevin Albertson Date: Thu, 20 Mar 2025 14:07:25 +0000 Subject: [PATCH 4/6] remove unused `client` params --- .../test/spec/client_side_encryption.cpp | 4 ++-- src/mongocxx/test/spec/command_monitoring.cpp | 5 +---- src/mongocxx/test/spec/retryable-reads.cpp | 4 ++-- src/mongocxx/test/spec/util.cpp | 10 +++++----- src/mongocxx/test/spec/util.hh | 18 +++++++++--------- src/mongocxx/test/versioned_api.cpp | 8 +++----- 6 files changed, 22 insertions(+), 27 deletions(-) diff --git a/src/mongocxx/test/spec/client_side_encryption.cpp b/src/mongocxx/test/spec/client_side_encryption.cpp index d2c1b1ca3e..1d93ec21be 100644 --- a/src/mongocxx/test/spec/client_side_encryption.cpp +++ b/src/mongocxx/test/spec/client_side_encryption.cpp @@ -194,7 +194,7 @@ void run_encryption_tests_in_file(std::string const& test_path) { auto tests = test_spec_view["tests"].get_array().value; /* we may not have a supported topology */ - CHECK_IF_SKIP_SPEC_TEST((client{uri{}, test_util::add_test_server_api()}), test_spec_view); + CHECK_IF_SKIP_SPEC_TEST(test_spec_view); mongocxx::client setup_client{ uri{}, @@ -208,7 +208,7 @@ void run_encryption_tests_in_file(std::string const& test_path) { auto const description = string::to_string(test["description"].get_string().value); DYNAMIC_SECTION(description) { - CHECK_IF_SKIP_SPEC_TEST((client{uri{}, test_util::add_test_server_api()}), test.get_document().value); + CHECK_IF_SKIP_SPEC_TEST(test.get_document().value); options::client client_opts; diff --git a/src/mongocxx/test/spec/command_monitoring.cpp b/src/mongocxx/test/spec/command_monitoring.cpp index 3f9f4011fd..51759465a1 100644 --- a/src/mongocxx/test/spec/command_monitoring.cpp +++ b/src/mongocxx/test/spec/command_monitoring.cpp @@ -70,10 +70,7 @@ void run_command_monitoring_tests_in_file(std::string test_path) { INFO("Test description: " << description); array::view expectations = test["expectations"].get_array().value; - // Use a separate client to check version info, so as not to interfere with APM - client temp_client{uri{}, test_util::add_test_server_api()}; - - CHECK_IF_SKIP_SPEC_TEST(temp_client, test.get_document().value); + CHECK_IF_SKIP_SPEC_TEST(test.get_document().value); // Used by the listeners auto events = expectations.begin(); diff --git a/src/mongocxx/test/spec/retryable-reads.cpp b/src/mongocxx/test/spec/retryable-reads.cpp index 7ac0d4b9e1..cfb9429199 100644 --- a/src/mongocxx/test/spec/retryable-reads.cpp +++ b/src/mongocxx/test/spec/retryable-reads.cpp @@ -48,10 +48,10 @@ void run_retryable_reads_tests_in_file(std::string test_path) { document::view test_spec_view = test_spec->view(); for (auto&& test : test_spec_view["tests"].get_array().value) { client client{get_uri(test.get_document().value), client_opts}; - CHECK_IF_SKIP_SPEC_TEST(client, test_spec_view); + CHECK_IF_SKIP_SPEC_TEST(test_spec_view); INFO("Test description: " << test["description"].get_string().value); - CHECK_IF_SKIP_SPEC_TEST(client, test.get_document()); + CHECK_IF_SKIP_SPEC_TEST(test.get_document()); auto get_value_or_default = [&](std::string key, std::string default_str) { if (test_spec_view[key]) { diff --git a/src/mongocxx/test/spec/util.cpp b/src/mongocxx/test/spec/util.cpp index 0b3ebd9014..8b137cb13c 100644 --- a/src/mongocxx/test/spec/util.cpp +++ b/src/mongocxx/test/spec/util.cpp @@ -92,7 +92,7 @@ uint32_t error_code_from_name(string_view name) { } /* Called with the entire test file and individual tests. */ -bool check_if_skip_spec_test_impl(client const& client, document::view test, std::string& reason) { +bool check_if_skip_spec_test_impl(document::view test, std::string& reason) { static std::set const unsupported_tests = { "CreateIndex and dropIndex omits default write concern", "MapReduce omits default write concern", @@ -854,7 +854,7 @@ void run_transactions_tests_in_file(std::string const& test_path) { auto const tests = test_spec_view["tests"].get_array().value; /* we may not have a supported topology */ - CHECK_IF_SKIP_SPEC_TEST((client{uri{}, test_util::add_test_server_api()}), test_spec_view); + CHECK_IF_SKIP_SPEC_TEST(test_spec_view); for (auto&& test : tests) { auto const description = string::to_string(test["description"].get_string().value); @@ -863,7 +863,7 @@ void run_transactions_tests_in_file(std::string const& test_path) { client setup_client{get_uri(test.get_document().value), test_util::add_test_server_api()}; // Step 1: If the `skipReason` field is present, skip this test completely. - CHECK_IF_SKIP_SPEC_TEST(setup_client, test.get_document().value); + CHECK_IF_SKIP_SPEC_TEST(test.get_document().value); // Steps 2-8. test_setup(test.get_document().value, test_spec_view); @@ -1007,13 +1007,13 @@ void run_crud_tests_in_file(std::string const& test_path, uri test_uri) { client client{std::move(test_uri), test_util::add_test_server_api(client_opts)}; document::view test_spec_view = test_spec->view(); - CHECK_IF_SKIP_SPEC_TEST(client, test_spec_view); + CHECK_IF_SKIP_SPEC_TEST(test_spec_view); for (auto&& test : test_spec_view["tests"].get_array().value) { auto description = test["description"].get_string().value; DYNAMIC_SECTION(to_string(description)) { - CHECK_IF_SKIP_SPEC_TEST(client, test.get_document()); + CHECK_IF_SKIP_SPEC_TEST(test.get_document()); auto get_value_or_default = [&](std::string key, std::string default_str) { if (test_spec_view[key]) { diff --git a/src/mongocxx/test/spec/util.hh b/src/mongocxx/test/spec/util.hh index 1e9cd15218..1986ee927c 100644 --- a/src/mongocxx/test/spec/util.hh +++ b/src/mongocxx/test/spec/util.hh @@ -36,15 +36,15 @@ using namespace mongocxx; /// uint32_t error_code_from_name(bsoncxx::stdx::string_view name); -bool check_if_skip_spec_test_impl(client const& client, document::view test, std::string& reason); - -#define CHECK_IF_SKIP_SPEC_TEST(client, test) \ - { \ - std::string reason; \ - if (mongocxx::spec::check_if_skip_spec_test_impl(client, test, reason)) { \ - SKIP(reason); \ - } \ - } \ +bool check_if_skip_spec_test_impl(document::view test, std::string& reason); + +#define CHECK_IF_SKIP_SPEC_TEST(test) \ + { \ + std::string reason; \ + if (mongocxx::spec::check_if_skip_spec_test_impl(test, reason)) { \ + SKIP(reason); \ + } \ + } \ ((void)0) /// diff --git a/src/mongocxx/test/versioned_api.cpp b/src/mongocxx/test/versioned_api.cpp index 6a092fab51..c5ebcf2835 100644 --- a/src/mongocxx/test/versioned_api.cpp +++ b/src/mongocxx/test/versioned_api.cpp @@ -29,15 +29,13 @@ using namespace mongocxx; -static bool has_api_version_1( - mongocxx::client const& client = mongocxx::client(uri(), test_util::add_test_server_api())) { +static bool has_api_version_1() { // API Version 1 was introduced in 5.0. return test_util::get_max_wire_version() >= 13; } -static bool has_api_version_1_with_count( - mongocxx::client const& client = mongocxx::client(uri(), test_util::add_test_server_api())) { - if (!has_api_version_1(client)) { +static bool has_api_version_1_with_count() { + if (!has_api_version_1()) { return false; } From db009f1274b3a3d1016cff4f6fb8cd8589fd0b66 Mon Sep 17 00:00:00 2001 From: Kevin Albertson Date: Thu, 3 Apr 2025 14:03:12 +0000 Subject: [PATCH 5/6] add missing server api version --- src/mongocxx/test/client_helpers.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mongocxx/test/client_helpers.cpp b/src/mongocxx/test/client_helpers.cpp index 5a48a7eeef..4b147164e8 100644 --- a/src/mongocxx/test/client_helpers.cpp +++ b/src/mongocxx/test/client_helpers.cpp @@ -57,7 +57,7 @@ namespace { // These frequently used network calls are cached to avoid bottlenecks during tests. document::value get_is_master() { static auto reply = []() { - auto client = mongocxx::client(mongocxx::uri()); + auto client = mongocxx::client{mongocxx::uri{}, test_util::add_test_server_api()}; return client["admin"].run_command(make_document(kvp("isMaster", 1))); }(); return reply; @@ -65,7 +65,7 @@ document::value get_is_master() { document::value get_server_status() { static auto status = []() { - auto client = mongocxx::client(mongocxx::uri()); + auto client = mongocxx::client{mongocxx::uri{}, test_util::add_test_server_api()}; return client["admin"].run_command(make_document(kvp("serverStatus", 1))); }(); return status; @@ -73,7 +73,7 @@ document::value get_server_status() { bsoncxx::stdx::optional get_shards() { static auto shards = []() { - auto client = mongocxx::client(mongocxx::uri()); + 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{}; From 2da7919b2ded256b03e9e78a7d92b3427ac5e4f3 Mon Sep 17 00:00:00 2001 From: Kevin Albertson Date: Thu, 3 Apr 2025 16:28:40 -0400 Subject: [PATCH 6/6] Add missing server API version Co-authored-by: Ezra Chung <88335979+eramongodb@users.noreply.github.com> --- src/mongocxx/test/client_helpers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mongocxx/test/client_helpers.cpp b/src/mongocxx/test/client_helpers.cpp index 4b147164e8..16669bf649 100644 --- a/src/mongocxx/test/client_helpers.cpp +++ b/src/mongocxx/test/client_helpers.cpp @@ -279,7 +279,7 @@ std::string get_server_version() { document::value get_server_params() { // Cache reply. static auto reply = []() { - auto client = mongocxx::client(mongocxx::uri()); + auto client = mongocxx::client{mongocxx::uri{}, test_util::add_test_server_api()}; return client["admin"].run_command(make_document(kvp("getParameter", "*"))); }();