Skip to content

PYTHON-6167 Split the test-non-standard task family into per-suite task families - #3131

Draft
blink1073 wants to merge 7 commits into
mongodb:mainfrom
blink1073:PYTHON-6167
Draft

blink1073 wants to merge 7 commits into
mongodb:mainfrom
blink1073:PYTHON-6167

Conversation

@blink1073

@blink1073 blink1073 commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Summary

Splits the shared test-non-standard Evergreen task family into one family per suite, so a task's name and tags identify the suite it runs. The encryption and load balancer matrices now run as separate sync and async tasks.

  • Before: 1325 tasks across 76 build variants
  • After: 833 tasks across 63 build variants

Motivation

The test-non-standard family was shared by the encryption, load balancer, doctest, and PyOpenSSL suites, and only the build variant decided which suite a task actually ran. That made failures hard to attribute.

Additionally, we were not testing sync and async separately for encryption or load balancer, and we were only testing load balancer with its own marker. The load balancer tests are meant to run the whole suite.

Changes

  • Replaces test-non-standard with one family per suite; a task's name identifies its suite, and distro tags decide which variant runs it.
  • Selects the sync or async half of a suite from a SYNC environment variable.
  • Builds OpenSSL 3 on RHEL 8 so free-threaded PyOpenSSL cells can run there, using the updated drivers-evergreen-tools submodule.

Testing

  • python3 -m pytest .evergreen/scripts/test_generate_config.py passes (21 tests), covering the new suite-identity, sync/async-split, and matrix-size invariants.
  • New tests pass in Evergreen: XXXX Patch URL

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved critical and moderate issues affect shell safety and test correctness.

2 open findings
What changed in this PR

Splits the shared Evergreen non-standard test family into suite-specific families with sync/async selection and updated platform matrices.

Changes:

  • Generates per-suite task families and selectors.
  • Adds sync/async filtering and OpenSSL 3 setup.
  • Regenerates Evergreen variants, tasks, and functions.
File Summary
.evergreen/​scripts/​utils.py Adds --sync; moderate issue (1 vote) with positional argument handling.
.evergreen/​scripts/​test_generate_config.py Adds matrix and suite-shape assertions.
.evergreen/​scripts/​setup_tests.py Adds sync filtering and OpenSSL setup; critical shell-injection issue (2 votes).
.evergreen/​scripts/​generate_config.py Generates suite-specific tasks; moderate issues (2, 1, and 1 votes) affecting load-balancer splitting, disabled test commands, and compressor propagation.
.evergreen/​generated_configs/​variants.yml Updates build-variant selectors.
.evergreen/​generated_configs/​tasks.yml Regenerates suite-specific task matrices.
.evergreen/​generated_configs/​functions.yml Forwards new task environment variables.

🧠 Review effort: Lite


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .evergreen/scripts/setup_tests.py
Comment thread .evergreen/scripts/generate_config.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved critical and moderate findings affect test coverage and task configuration.

2 open findings
2 resolved since last review

🧠 Review effort: Lite

Comment thread .evergreen/scripts/generate_config.py Outdated
Comment thread .evergreen/scripts/utils.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Three moderate review findings remain unresolved in task generation and configuration.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Allowlist TEST_CRYPT_SHARED for crypt_shared test tasks

.evergreen/​scripts/​generate_config.py:1615

The crypt_shared tasks now put TEST_CRYPT_SHARED in the run tests command vars, but the environment allowlist still omits it. setup_tests.py reads this value to enable the crypt_shared setup, so the new crypt_shared tasks will run without that configuration and test the regular libmongocrypt path instead. Add TEST_CRYPT_SHARED to this allowlist.

2 resolved since last review

Review effort: Lite

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Address the critical environment-value shell-injection issue and preserve TEST_CRYPT_SHARED for encryption tests.

2 open issues

Review effort: Lite

Comment on lines +200 to +204
for line in env.splitlines():
name, _, value = line.partition("=")
if name in ("OPENSSL_DIR", "LD_LIBRARY_PATH"):
os.environ[name] = value
write_env(name, value)
expansions.update(extra_expansions)
if crypt_shared:
tags.append("crypt-shared")
expansions["TEST_CRYPT_SHARED"] = "true"

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved critical and moderate findings affect shell safety, option parsing, test coverage, and compression execution.

4 open issues

Review effort: Lite

CRYPT_SHARED_DIR = Path(config["CRYPT_SHARED_LIB_PATH"]).parent.as_posix()
LOGGER.info("Using crypt_shared_dir %s", CRYPT_SHARED_DIR)
# The trailing expansions must stay live; escape the literal prefix.
escaped_dir = CRYPT_SHARED_DIR.replace("$", "\\$").replace("`", "\\`")
value = str(value).replace('"', "")
if escape:
# Double quotes expand `$` and backticks: keep the value literal.
value = value.replace("$", "\\$").replace("`", "\\`")

This branch has not been deployed

No deployments
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.

2 participants