Skip to content

CDRIVER-4537 Ensure all aborts in test suite are preceeded by flushes #1164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 16, 2022
Merged

CDRIVER-4537 Ensure all aborts in test suite are preceeded by flushes #1164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
88f5fa2
CDRIVER-4537 Ensure all aborts in test suite are preceeded by flushes
eramongodb Dec 14, 2022
ade7659
Add else statement to ensure proper macro statement expansion
eramongodb Dec 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build/future_function_templates/future-value.h.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ typedef struct _future_value_t
} value;
} future_value_t;

future_value_t *future_value_new ();
future_value_t *future_value_new (void);

#ifdef __clang__
#pragma clang diagnostic push
Expand Down
22 changes: 14 additions & 8 deletions build/future_function_templates/future.c.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,23 @@

{{ header_comment }}

#define FUTURE_TIMEOUT_ABORT \
if (1) { \
fflush (stdout); \
fprintf (stderr, "%s timed out\n", BSON_FUNC); \
fflush (stderr); \
abort (); \
} else \
((void) 0)
Comment on lines +10 to +17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why we use this if (1) { ... } else pattern in several places rather than the more common do { ... } while (0)?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that pattern started recently from @vector-of-bool. This article describes the motivation.


void
future_get_void (future_t *future)
{
if (!future_wait (future)) {
fprintf (stderr, "%s timed out\n", BSON_FUNC);
fflush (stderr);
abort ();
if (future_wait (future)) {
return;
}

FUTURE_TIMEOUT_ABORT;
}

{% for T in type_list %}
Expand All @@ -25,9 +34,7 @@ future_get_{{ T }} (future_t *future)
return future_value_get_{{ T }} (&future->return_value);
}

fprintf (stderr, "%s timed out\n", BSON_FUNC);
fflush (stderr);
abort ();
FUTURE_TIMEOUT_ABORT;
}
{% endfor %}

Expand Down Expand Up @@ -120,4 +127,3 @@ future_destroy (future_t *future)
bson_mutex_destroy (&future->mutex);
bson_free (future);
}

13 changes: 7 additions & 6 deletions src/libmongoc/tests/TestSuite.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,13 @@ bson_open (const char *filename, int flags, ...)
void
_test_error (const char *format, ...) BSON_GNUC_PRINTF (1, 2);

#define test_error(...) \
if (1) { \
MONGOC_STDERR_PRINTF ( \
"test error in: %s %d:%s()\n", __FILE__, __LINE__, BSON_FUNC); \
_test_error (__VA_ARGS__); \
} else \
#define test_error(...) \
if (1) { \
MONGOC_STDERR_PRINTF ( \
"test error in: %s %d:%s()\n", __FILE__, __LINE__, BSON_FUNC); \
_test_error (__VA_ARGS__); \
abort (); /* suppress missing return errors in non-void functions */ \
} else \
((void) 0)

#define bson_eq_bson(bson, expected) \
Expand Down
9 changes: 3 additions & 6 deletions src/libmongoc/tests/json-test-monitoring.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,9 @@ assert_host_in_uri (const mongoc_host_list_t *host, const mongoc_uri_t *uri)
hosts = hosts->next;
}

fprintf (stderr,
"Host \"%s\" not in \"%s\"",
host->host_and_port,
mongoc_uri_get_string (uri));
fflush (stderr);
abort ();
test_error ("Host \"%s\" not in \"%s\"",
host->host_and_port,
mongoc_uri_get_string (uri));
}


Expand Down
5 changes: 1 addition & 4 deletions src/libmongoc/tests/json-test-operations.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ session_from_name (json_test_ctx_t *ctx, const char *session_name)
} else if (!strcmp (session_name, "session1")) {
return ctx->sessions[1];
} else {
MONGOC_ERROR ("Unrecognized session name: %s", session_name);
abort ();
test_error ("Unrecognized session name: %s", session_name);
}
}

Expand Down Expand Up @@ -794,7 +793,6 @@ add_request_to_bulk (mongoc_bulk_operation_t *bulk,
bulk, &filter, &update, &opts, error);
} else {
test_error ("unrecognized request name %s", name);
abort ();
}

bson_destroy (&opts);
Expand Down Expand Up @@ -957,7 +955,6 @@ single_write (mongoc_collection_t *collection,
collection, &filter, &update, &opts, reply, &error);
} else {
test_error ("unrecognized request name %s", name);
abort ();
}

value_init_from_doc (&value, reply);
Expand Down
63 changes: 25 additions & 38 deletions src/libmongoc/tests/json-test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,6 @@ test_server_selection_logic_cb (bson_t *test)
sd->round_trip_time_msec = bson_iter_int32 (&sd_iter);
} else if (sd->type != MONGOC_SERVER_UNKNOWN) {
test_error ("%s has no avg_rtt_ms", sd->host.host_and_port);
abort ();
}

if (bson_iter_init_find (&sd_iter, &server, "maxWireVersion")) {
Expand Down Expand Up @@ -529,7 +528,6 @@ test_server_selection_logic_cb (bson_t *test)
if (!found) {
test_error ("Should have been selected but wasn't: %s",
bson_iter_utf8 (&host, NULL));
abort ();
}

matched_servers[i] = true;
Expand All @@ -543,7 +541,6 @@ test_server_selection_logic_cb (bson_t *test)

test_error ("Shouldn't have been selected but was: %s",
sd->host.host_and_port);
abort ();
}
}

Expand Down Expand Up @@ -652,9 +649,9 @@ collect_tests_from_dir (char (*paths)[MAX_TEST_NAME_LENGTH] /* OUT */,

dir = opendir (dir_path);
if (!dir) {
MONGOC_ERROR ("Cannot open \"%s\"", dir_path);
MONGOC_ERROR ("Run test-libmongoc in repository root directory.\n");
abort ();
test_error ("Cannot open \"%s\"\n"
"Run test-libmongoc in repository root directory.",
dir_path);
}

while ((entry = readdir (dir))) {
Expand Down Expand Up @@ -720,7 +717,7 @@ get_bson_from_json_file (char *filename)
/* read entire file into buffer */
buffer = (const char *) bson_malloc0 (length);
if (fread ((void *) buffer, 1, length, file) != length) {
abort ();
test_error ("Failed to read JSON file into buffer");
}

fclose (file);
Expand All @@ -731,8 +728,7 @@ get_bson_from_json_file (char *filename)
/* convert to bson */
data = bson_new_from_json ((const uint8_t *) buffer, length, &error);
if (!data) {
fprintf (stderr, "Cannot parse %s: %s\n", filename, error.message);
abort ();
test_error ("Cannot parse %s: %s", filename, error.message);
}

bson_free ((void *) buffer);
Expand Down Expand Up @@ -1430,10 +1426,9 @@ set_uri_opts_from_bson (mongoc_uri_t *uri, const bson_t *opts)
} else if (mongoc_uri_option_is_utf8 (key)) {
mongoc_uri_set_option_as_utf8 (uri, key, bson_iter_utf8 (&iter, NULL));
} else {
MONGOC_ERROR ("Unsupported clientOptions field \"%s\" in %s",
key,
bson_as_json (opts, NULL));
abort ();
test_error ("Unsupported clientOptions field \"%s\" in %s",
key,
bson_as_json (opts, NULL));
}
}
}
Expand Down Expand Up @@ -1479,11 +1474,10 @@ set_auto_encryption_opts (mongoc_client_t *client, bson_t *test)
test_framework_getenv ("MONGOC_TEST_AWS_ACCESS_KEY_ID");

if (!secret_access_key || !access_key_id) {
fprintf (stderr,
"Set MONGOC_TEST_AWS_SECRET_ACCESS_KEY and "
"MONGOC_TEST_AWS_ACCESS_KEY_ID environment variables to "
"run Client Side Encryption tests.\n");
abort ();
test_error (
"Set MONGOC_TEST_AWS_SECRET_ACCESS_KEY and "
"MONGOC_TEST_AWS_ACCESS_KEY_ID environment variables to "
"run Client Side Encryption tests.");
}

{
Expand Down Expand Up @@ -1517,19 +1511,16 @@ set_auto_encryption_opts (mongoc_client_t *client, bson_t *test)
test_framework_getenv ("MONGOC_TEST_AWS_TEMP_SESSION_TOKEN");

if (!secret_access_key || !access_key_id) {
fprintf (stderr,
"Set MONGOC_TEST_AWS_TEMP_SECRET_ACCESS_KEY and "
"MONGOC_TEST_AWS_TEMP_ACCESS_KEY_ID environment variables "
"to run Client Side Encryption tests.\n");
abort ();
test_error (
"Set MONGOC_TEST_AWS_TEMP_SECRET_ACCESS_KEY and "
"MONGOC_TEST_AWS_TEMP_ACCESS_KEY_ID environment variables "
"to run Client Side Encryption tests.");
}

// Only require session token when requested.
if (!session_token && need_aws_with_session_token) {
fprintf (stderr,
"Set MONGOC_TEST_AWS_TEMP_SESSION_TOKEN environment "
"variable to run Client Side Encryption tests.\n");
abort ();
test_error ("Set MONGOC_TEST_AWS_TEMP_SESSION_TOKEN environment "
"variable to run Client Side Encryption tests.");
}

{
Expand Down Expand Up @@ -1567,12 +1558,10 @@ set_auto_encryption_opts (mongoc_client_t *client, bson_t *test)
test_framework_getenv ("MONGOC_TEST_AZURE_CLIENT_SECRET");

if (!azure_tenant_id || !azure_client_id || !azure_client_secret) {
fprintf (stderr,
"Set MONGOC_TEST_AZURE_TENANT_ID, "
"MONGOC_TEST_AZURE_CLIENT_ID, and "
"MONGOC_TEST_AZURE_CLIENT_SECRET to enable CSFLE "
"tests.");
abort ();
test_error ("Set MONGOC_TEST_AZURE_TENANT_ID, "
"MONGOC_TEST_AZURE_CLIENT_ID, and "
"MONGOC_TEST_AZURE_CLIENT_SECRET to enable CSFLE "
"tests.");
}

bson_concat (&kms_providers,
Expand All @@ -1595,11 +1584,9 @@ set_auto_encryption_opts (mongoc_client_t *client, bson_t *test)
gcp_privatekey = test_framework_getenv ("MONGOC_TEST_GCP_PRIVATEKEY");

if (!gcp_email || !gcp_privatekey) {
fprintf (stderr,
"Set MONGOC_TEST_GCP_EMAIL and "
"MONGOC_TEST_GCP_PRIVATEKEY to enable CSFLE "
"tests.");
abort ();
test_error ("Set MONGOC_TEST_GCP_EMAIL and "
"MONGOC_TEST_GCP_PRIVATEKEY to enable CSFLE "
"tests.");
}

bson_concat (
Expand Down
Loading