Skip to content

CDRIVER-5901 support auto encryption in unified tests #2004

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.

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 21 commits into from
Jun 6, 2025

Conversation

mdb-ad
Copy link
Contributor

@mdb-ad mdb-ad commented May 1, 2025

@mdb-ad mdb-ad force-pushed the unified-tests-only branch from 5ede743 to 488c3e5 Compare May 5, 2025 06:01
@mdb-ad mdb-ad force-pushed the unified-tests-only branch from 488c3e5 to da08915 Compare May 5, 2025 06:06
@mdb-ad mdb-ad requested a review from kevinAlbs May 5, 2025 06:08
@mdb-ad mdb-ad marked this pull request as ready for review May 16, 2025 22:58
@mdb-ad mdb-ad requested a review from a team as a code owner May 16, 2025 22:58
@mdb-ad mdb-ad requested review from eramongodb and kevinAlbs May 16, 2025 22:58
@mdb-ad mdb-ad requested a review from eramongodb May 29, 2025 02:27
Copy link
Contributor

@eramongodb eramongodb left a comment

Choose a reason for hiding this comment

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

Aside from addressing ClangFormat issues, LGTM.

I suspect EVG tasks are currently failing due to DRIVERS-3188, not due to changes in this PR.

Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

On second look, I think some test task failures may be introduced in this PR. For example asan-sasl-cyrus-openssl-ubuntu2004-clang-test-latest-sharded-auth is failing with:

error: missing required environment variable 'MONGOC_TEST_AWS_ACCESS_KEY_ID'

I expect that task is not intended to run client side encryption tests (e.g. tasks named *-cse-* run client side encryption tests). The unified tests may need to be conditioned on test_framework_skip_if_no_client_side_encryption:

diff --git a/src/libmongoc/tests/unified/runner.c b/src/libmongoc/tests/unified/runner.c
index b049d464ef..81b052e544 100644
--- a/src/libmongoc/tests/unified/runner.c
+++ b/src/libmongoc/tests/unified/runner.c
@@ -794,6 +794,11 @@ check_run_on_requirement (test_runner_t *test_runner,
             return false;
          }
 
+         if (0 == test_framework_skip_if_no_client_side_encryption ()) {
+            *fail_reason = bson_strdup_printf ("CSFLE is required but not all environment variables are set");
+            return false;
+         }
+
          if (csfle_required) {
             continue;
          }

@mdb-ad mdb-ad requested a review from kevinAlbs May 29, 2025 20:01
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

LGTM (assuming expected tests pass).

@mdb-ad mdb-ad merged commit 957868c into mongodb:master Jun 6, 2025
29 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants