Make script and OIDC logout tests robust to serial (-n0) execution#3507
Conversation
These tests pass under the normal xdist test run but fail when the suite runs serially with pytest-xdist disabled (-n0), which is how the backwards-compatibility CI check exercises a previous release's suite. The three script tests constructed a Script with no cmd_args and called do_run(), so the script's argparse fell back to sys.argv[1:]. Under xdist each worker has a clean execnet argv, but under -n0 sys.argv is the real pytest command line, so argparse either errored on unrecognized arguments (equivalents, novelist) or silently matched a flag -- the leaked '-m db' marker tripped RebuildSearchIndexScript's -m/--migration flag, sending it down the migration path so search_reindex.delay was never called. Pass an explicit cmd_args=[] to express 'no arguments' independent of sys.argv. The OIDC logout test asserted on caplog without setting a capture level. Under serial execution the effective log level is left raised by an earlier test, so the warning was not captured. Set the level explicitly, matching the sibling test_oidc_logout_initiate_revocation_only.
|
Claude finished @jonathangreen's task in 1m 28s —— View job Review
SummaryThis is a clean, well-scoped test-isolation fix and the PR description accurately diagnoses each failure. I traced all four changes against the code under test and they are each correct and placed at the right call site. No production code is touched, and the asymmetry in where For the record, the three script call sites line up with the parsing location in each script:
And the OIDC change is sound: the message is logged at |
Greptile SummaryThis PR fixes four test-isolation bugs that were masked by pytest-xdist parallel execution and surfaced when tests are run serially (
Confidence Score: 5/5All changes are confined to test files; no production behaviour is altered. Each change is a single-line, targeted isolation fix. The script tests now explicitly pass cmd_args=[] so they no longer depend on sys.argv, and the OIDC test now owns its capture level rather than inheriting whatever a previous test left behind. All four fixes are consistent with the patterns already established in neighbouring tests in the same files. No files require special attention. Important Files Changed
Reviews (1): Last reviewed commit: "Make script and OIDC logout tests robust..." | Re-trigger Greptile |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3507 +/- ##
=======================================
Coverage 93.44% 93.44%
=======================================
Files 511 511
Lines 46465 46465
Branches 6339 6339
=======================================
+ Hits 43419 43421 +2
+ Misses 1969 1968 -1
+ Partials 1077 1076 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
Description
Makes four tests robust to running serially with pytest-xdist disabled (
-n0). They pass under the normal xdist test run but fail under-n0, which is how the backwards-compatibility CI check (#3506) runs a previous release's database suite against the current schema.test_equivalents,test_novelist,test_search) constructed aScriptwith nocmd_argsand calleddo_run(), so the script's argparse fell back tosys.argv[1:]. Under xdist each worker has a clean execnetsys.argv, but under-n0sys.argvis the real pytest command line. That caused argparse to error on unrecognized arguments (equivalents,novelist), and the leaked-m dbmarker matchedRebuildSearchIndexScript's-m/--migrationflag, sending it down the migration path sosearch_reindex.delaywas never called (search). Each now passes an explicitcmd_args=[]to express "no arguments" independent ofsys.argv.test_oidc_logout_initiate_no_stored_id_token) asserted oncaplogwithout setting a capture level. Under serial execution the effective log level is left raised by an earlier test, so the warning was not captured andcaplog.textwas empty. It now sets the level explicitly, matching its passing siblingtest_oidc_logout_initiate_revocation_only.No production code changes — scripts intentionally default
cmd_argstosys.argvfor real CLI use viaScript.run().Motivation and Context
The backwards-compatibility check added in #3506 runs the previous release's
-m dbsuite under-n0(external-schema mode requires serial execution — the single shared database can't be used by parallel xdist workers). Its first real run surfaced these four latent test-isolation bugs, which xdist had been masking. None are schema-compatibility failures; all four fail identically against the previous release's own schema. Fixing them here means that once a release containing these fixes becomes the "previous release", the check will pass against it.How Has This Been Tested?
Reproduced the failures on the unfixed tests by running them under
-n0in the docker test environment (tox -e py312-docker -- -n0 ...), observing the argparseunrecognized argumentserrors. With the fixes applied, all four pass under-n0(including with-m dbpresent, which exercises thesearchflag-collision case):60 passed, 19 deselected.Checklist