jesse + jeremy comments

- update handling of useMultipleMongoses in UTF
- use mongoc_ prefix in mock global
- clang format
- clean up if/else branching

Author: kevinAlbs

src/libmongoc/src/mongoc/mongoc-server-description-private.h

@@ -200,6 +200,10 @@ bool
 mongoc_server_description_has_service_id (
    const mongoc_server_description_t *description);
 
-extern bool global_mock_service_id;
+/* mongoc_global_mock_service_id is only used for testing. The test runner sets
+ * this to true when testing against a load balanced deployment to mock the
+ * presence of a serviceId field in the "hello" response. The purpose of this is
+ * further described in the Load Balancer test README. */
+extern bool mongoc_global_mock_service_id;
 
 #endif

src/libmongoc/src/mongoc/mongoc-server-description.c

@@ -33,7 +33,7 @@ static bson_oid_t kObjectIdZero = {{0}};
 
 const bson_oid_t kZeroServiceId = {{0}};
 
-bool global_mock_service_id = false;
+bool mongoc_global_mock_service_id = false;
 
 static bool
 _match_tag_set (const mongoc_server_description_t *sd,
@@ -738,7 +738,7 @@ mongoc_server_description_handle_hello (mongoc_server_description_t *sd,
    }
 
 
-   if (global_mock_service_id) {
+   if (mongoc_global_mock_service_id) {
       bson_iter_t pid_iter;
 
       if (bson_iter_init_find (&pid_iter, &sd->topology_version, "processId") &&

src/libmongoc/tests/TestSuite.c

@@ -886,24 +886,21 @@ bool
 test_matches (TestSuite *suite, Test *test)
 {
    int i;
-   bool matches;
 
    /* If no match patterns were provided, then assume all match. */
    if (suite->match_patterns.len == 0) {
-      matches = true;
-   } else {
-      matches = false;
-      for (i = 0; i < suite->match_patterns.len; i++) {
-         char *pattern =
-            _mongoc_array_index (&suite->match_patterns, char *, i);
-         if (TestSuite_TestMatchesName (suite, test, pattern)) {
-            matches = true;
-            break;
-         }
+      return true;
+   }
+
+   for (i = 0; i < suite->match_patterns.len; i++) {
+      char *pattern =
+         _mongoc_array_index (&suite->match_patterns, char *, i);
+      if (TestSuite_TestMatchesName (suite, test, pattern)) {
+         return true;
       }
    }
 
-   return matches;
+   return false;
 }
 
 static int

src/libmongoc/tests/test-libmongoc.c

@@ -1408,13 +1408,16 @@ test_framework_get_uri_multi_mongos_loadbalanced () {
    char *uri_str_no_auth;
    char *uri_str;
    mongoc_uri_t *uri;
+   bson_error_t error;
 
    uri_str_no_auth = _mongoc_getenv ("MULTI_MONGOS_LB_URI");
    if (!uri_str_no_auth) {
       test_error ("expected MULTI_MONGOS_LB_URI to be set");
    }
    uri_str = test_framework_add_user_password_from_env (uri_str_no_auth);
-   uri = mongoc_uri_new (uri_str);
+   uri = mongoc_uri_new_with_error (uri_str, &error);
+
+   ASSERT_OR_PRINT (uri, error);
 
    bson_free (uri_str_no_auth);
    bson_free (uri_str);
@@ -2951,7 +2954,7 @@ main (int argc, char *argv[])
    test_generation_map_install (&suite);
 
    if (test_framework_is_loadbalanced ()) {
-      global_mock_service_id = true;
+      mongoc_global_mock_service_id = true;
    }
 
    ret = TestSuite_Run (&suite);

src/libmongoc/tests/test-mongoc-loadbalanced.c

@@ -774,10 +774,7 @@ test_post_handshake_error_clears_pool (void)
 static int
 skip_if_not_loadbalanced (void)
 {
-   if (!test_framework_is_loadbalanced ()) {
-      return 0;
-   }
-   return 1;
+   return test_framework_is_loadbalanced () ? 1 : 0;
 }
 
 void

src/libmongoc/tests/test-mongoc-retryable-writes.c

@@ -35,7 +35,7 @@ retryable_writes_test_run_operation (json_test_ctx_t *ctx,
 static test_skip_t skips[] = {
    {"InsertOne fails after multiple retryable writeConcernErrors",
     "Waiting on CDRIVER-3790"},
-   {0, 0}};
+   {0}};
 
 /* Callback for JSON tests from Retryable Writes Spec */
 static void

src/libmongoc/tests/unified/entity-map.c

@@ -343,15 +343,21 @@ entity_client_new (entity_map_t *em, bson_t *bson, bson_error_t *error)
     * host. If useMultipleMongoses is false, require that the connection string
     * has one host. If useMultipleMongoses unspecified, make no assertion.
     */
-   if (use_multiple_mongoses != NULL) {
-      if (*use_multiple_mongoses && test_framework_is_loadbalanced ()) {
+   if (test_framework_is_loadbalanced ()) {
+      /* Quoting the unified test runner specification:
+       * If the topology type is LoadBalanced, [...] If useMultipleMongoses is
+       * true or unset, the test runner MUST use the URI of the load balancer
+       * fronting multiple servers. Otherwise, the test runner MUST use the URI
+       * of the load balancer fronting a single server.
+       */
+      if (use_multiple_mongoses == NULL || *use_multiple_mongoses == true) {
          mongoc_uri_destroy (uri);
          uri = test_framework_get_uri_multi_mongos_loadbalanced ();
-      } else {
-         if (!test_framework_uri_apply_multi_mongos (
-                uri, *use_multiple_mongoses, error)) {
-            goto done;
-         }
+      }
+   } else if (use_multiple_mongoses != NULL) {
+      if (!test_framework_uri_apply_multi_mongos (
+               uri, *use_multiple_mongoses, error)) {
+         goto done;
       }
    }
 

src/libmongoc/tests/unified/runner.c

@@ -381,7 +381,7 @@ test_runner_new (void)
    uri = test_framework_get_uri ();
    /* In load balanced mode, the internal client must use the SINGLE_LB_MONGOS_URI. */
    if (!test_framework_is_loadbalanced ()) {
-      /* Always use multiple mongos's if speaking to a mongos.
+      /* Always use multiple mongoses if speaking to a mongos.
        * Some test operations require communicating with all known mongos */
       if (!test_framework_uri_apply_multi_mongos (uri, true, &error)) {
          test_error ("error applying multiple mongos: %s", error.message);
@@ -1151,9 +1151,7 @@ test_check_expected_events_for_client (test_t *test,
           * supported. */
          ret = true;
          goto done;
-      } else if (0 == strcmp (event_type, "command")) {
-         /* proceed. */
-      } else {
+      } else if (0 != strcmp (event_type, "command")) {
          test_set_error (error, "unexpected event type: %s", event_type);
          goto done;
       }

src/libmongoc/tests/unified/util.c

@@ -166,23 +166,23 @@ test_bson_util_install (TestSuite *suite)
 
 /* TODO (CDRIVER-3525) add test support for CMAP events once the C driver
  * supports CMAP. */
-bool is_unsupported_event_type (const char* event_type) {
-   char * unsupported_event_types[] = {
-      "poolCreatedEvent",
-      "poolReadyEvent",
-      "poolClearedEvent",
-      "poolClosedEvent",
-      "connectionCreatedEvent",
-      "connectionReadyEvent",
-      "connectionClosedEvent",
-      "connectionCheckOutStartedEvent",
-      "connectionCheckOutFailedEvent",
-      "connectionCheckedOutEvent",
-      "connectionCheckedInEvent",
-      NULL
-   };
-
-   char ** iter;
+bool
+is_unsupported_event_type (const char *event_type)
+{
+   char *unsupported_event_types[] = {"poolCreatedEvent",
+                                      "poolReadyEvent",
+                                      "poolClearedEvent",
+                                      "poolClosedEvent",
+                                      "connectionCreatedEvent",
+                                      "connectionReadyEvent",
+                                      "connectionClosedEvent",
+                                      "connectionCheckOutStartedEvent",
+                                      "connectionCheckOutFailedEvent",
+                                      "connectionCheckedOutEvent",
+                                      "connectionCheckedInEvent",
+                                      NULL};
+
+   char **iter;
 
    for (iter = unsupported_event_types; *iter != NULL; iter++) {
       if (0 == strcmp (event_type, *iter)) {