PYTHON-6070 Improve help output for just run-server and just setup-tests - #3035
PYTHON-6070 Improve help output for just run-server and just setup-tests#3035blink1073 wants to merge 7 commits into
Conversation
just run-server -h now forwards to run-mongodb.sh start -h instead of printing generic argparse help, and just setup-tests <name> -h shows the valid sub_test_name choices for test names where they're enumerable (kms, auth_aws, auth_oidc, mod_wsgi, perf).
There was a problem hiding this comment.
🟡 Changes recommended
The updated argparse configuration can still produce misleading/duplicated help output and should be adjusted to match the PR’s stated help-output goals.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR (PYTHON-6070) improves the developer UX of Evergreen helper scripts by refining CLI help output and making just run-server -h delegate help to the underlying drivers-tools script.
Changes:
- Enhanced
setup-testshelp output by enumerating validsub_test_namechoices for specific test suites and reducing repeated/verbose choice listings. - Consolidated “sub-test required” logic into a single
SUB_TEST_NAME_MAPsource of truth. - Forwarded
run-server -h/--helptorun-mongodb.sh start -hinstead of printing the Python script’s argparse help.
File summaries
| File | Description |
|---|---|
| .evergreen/scripts/utils.py | Refactors CLI option parsing/help text for setup-tests, adds SUB_TEST_NAME_MAP, and reorganizes help sections. |
| .evergreen/scripts/run_server.py | Adds -h/--help forwarding behavior to drivers-tools run-mongodb.sh. |
Review details
Suppressed comments (1)
.evergreen/scripts/utils.py:142
sub_test_nameis validated as required for tests inSUB_TEST_NAME_MAP, but the generated help text always calls it "optional". Forjust setup-tests <name> -hthis is misleading because the command will error without a sub-test for these suites; the help can mention that it’s required for the pinned test name.
help_text = (
f"The optional sub test name, for example {example_sub_test_name!r}. "
f"One of: {', '.join(sub_test_choices)}."
)
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
6ab6840 to
7388125
Compare
Pass cwd=DRIVERS_TOOLS when forwarding -h to run-mongodb.sh, matching the normal start path. Clarify that sub_test_name is required, not optional, for test names in SUB_TEST_NAME_MAP.
There was a problem hiding this comment.
🟡 Changes recommended
The current argv-scanning logic in get_test_options() can mis-detect the intended test_name and break valid invocations, and run_server.py -h can crash when DRIVERS_TOOLS is unset instead of providing usable help.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The run-server -h/--help forwarding currently goes through run_command, which adds extra INFO log lines and prevents the help output from being forwarded verbatim as shown in the PR description.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
Only pin the test_name argument's choices from the first positional argument, not any token in argv, so a sub_test_name that happens to match a SUB_TEST_NAME_MAP key no longer gets misidentified as the test suite. Fail fast with a clear error when DRIVERS_TOOLS is unset instead of a confusing FileNotFoundError.
run_command logs "Running command..." before and after every call, which wrapped the forwarded run-mongodb.sh start -h output and defeated the point of showing it verbatim. Call subprocess.run directly for the -h path instead.
There was a problem hiding this comment.
🟡 Changes recommended
just run-server -h/--help currently fails when DRIVERS_TOOLS is unset due to an early env check that prevents reaching any help output.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
just run-server -h/--help was raising before reaching the help path when DRIVERS_TOOLS was unset. Only raise for the real start path; -h without DRIVERS_TOOLS now falls back to get_test_options' own argparse help instead of erroring.
There was a problem hiding this comment.
🔵 Needs a closer look
The new known_test_name inference can mis-detect the suite name when options with values are placed before the positional test name, leading to incorrect/help output behavior.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.evergreen/scripts/utils.py:111
- The logic that infers
known_test_nametreats any non-flag token as positional. This mis-detects the test name when an option that takes a value is provided before the test name (e.g.--green-framework gevent kms -h), becausegeventwill be treated as the first positional argument and the help output won’t be specialized (and could become wrong if an option value ever matches a test name). Consider skipping known option values when identifying the first positional argument.
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
An option that takes a value (e.g. --green-framework gevent) placed before the test_name positional was mistaken for it, silently skipping the help specialization. Skip known value-taking options' values when scanning for the first positional argument.
There was a problem hiding this comment.
🔵 Needs a closer look
run-server -h can currently fail if DRIVERS_TOOLS is set to a stale/invalid path, instead of falling back to local argparse help.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.evergreen/scripts/run_server.py:25
- When
-h/--helpis requested andDRIVERS_TOOLSis set but points to a non-existent checkout (orrun-mongodb.shis missing), this will raise an exception instead of falling back to the local argparse help. Consider verifyingDRIVERS_TOOLS/run-mongodb.shexist before forwarding so help remains reliable even with a stale env var.
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
A stale or invalid DRIVERS_TOOLS (pointing at a missing checkout or missing run-mongodb.sh) crashed with a confusing FileNotFoundError instead of falling back to the local argparse help. Only forward -h when run-mongodb.sh actually exists.
There was a problem hiding this comment.
🟢 Approval recommended
The changes are localized to developer/Evergreen scripts, align with the PR’s stated behavior, and do not introduce any verified functional regressions in argument parsing or validation.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
PYTHON-6070
Changes in this PR
just run-server -hnow forwards torun-mongodb.sh start -hinstead of printing generic argparse help forrun_server.py.just setup-tests <test_name> -hshows the validsub_test_namechoices for test names where they're enumerable (kms,auth_aws,auth_oidc,mod_wsgi,perf).SUB_TEST_REQUIREDintoSUB_TEST_NAME_MAPso the sub-test-required check and the choice list share one source of truth.setup-testsflags (--test-min-deps,--green-framework,--compressor,--crypt-shared,--no-ext,--mongodb-api-version,--disable-test-commands) under an "other options" help section.test_name/sub_test_nameusage/help output so the full choice list appears once instead of being repeated in both the usage line and the arguments section.Example:
just setup-tests kms -hExample:
just run-server -hrun_server.pynow forwards straight to the drivers-tools script instead of printing its own help:(This help text comes from
run-mongodb.shin drivers-evergreen-tools, not from this repo.)Test Plan
get_test_optionswithkms,auth_aws,auth_oidc,mod_wsgi,perf, and no test name, confirming help output and validation for each.just run-server -hagainst a local checkout of drivers-evergreen-tools to confirm it forwards to and printsrun-mongodb.sh start -h's own output.just lint-manual— all hooks pass.just typing— pre-existing unrelated failures inpymongo/ocsp_support.py, no new errors.pr(all 47pr-tagged variants) — completed with no task failures.Checklist
Checklist for Author
Checklist for Reviewer