gh-156031: Restore urllib.request._opener in test_httpservers - #156032
gh-156031: Restore urllib.request._opener in test_httpservers#156032ayaangazali wants to merge 3 commits into
Conversation
CommandLineRunTimeTestCase.fetch_file() calls urlopen() without an SSL context for the plain HTTP cases. urlopen() only builds a throwaway opener when a context is passed, so those calls install the urllib.request._opener module global and leave it set for the rest of the test run. test_urllib already guards the same global with addCleanup(urlcleanup) in several places; do the same here.
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
There was a problem hiding this comment.
Test only changes don't requires a news entry file.
|
Good catch, thanks. Removed the NEWS entry, the diff is back to just the one-line cleanup in the test. For context on why it was there: |
|
Introduced in 3.15, so only need to backport to 3.15 branch. |
test_httpserversleavesurllib.request._openerinstalled for the rest of the test run, so regrtest flags it as altering the execution environment. Details and the trace are in gh-156031.Short version:
CommandLineRunTimeTestCase.fetch_file()callsurlopen()with no SSL context for the plain HTTP cases, andurlopen()only builds a throwaway opener when a context is passed. Otherwise it assigns the module global:test_urllibalready guards the same global withself.addCleanup(urllib.request.urlcleanup)in four places, andurlcleanup()resets_opener, so this uses the same pattern rather than inventing a new one.Verifying
Repro takes about two seconds.
test_urllibruns first only to geturllib.requestintosys.modules, which is what makes regrtest watch the resource at all:Before:
After:
SUCCESS, same 209 tests run.Also ran the whole suite sequentially, which is the only mode that detects this, before and after:
test_httpservers)test_struct)test_struct)test_httpserversgoes fromfailed (env changed)topassed. Thetest_structfailure is unrelated and present before my change too, so I left it alone.pre-commitpasses on the touched file.Worth noting why this has not turned up in CI:
save_env.pyonly watches the resource whenurllib.requestis already imported, becausetry_get_module()raisesSkipTestEnvironmentotherwise. Under-jeach test file gets a fresh subprocess, so the module is not insys.moduleswhen regrtest snapshots the environment and the resource is never tracked. Runningtest_httpserverson its own is clean for the same reason. It only shows up sequentially, after something earlier has importedurllib.request.No test added. The fix is test-only hygiene and the suite run above is the check, so a new test would just restate it.
apologies if any of this is off. I found it by running the suite sequentially, traced the cause myself, and talked the fix choice over with Claude Code before settling on matching the existing
test_urllibpattern. happy to be corrected on any of it. freshman in college, doing what I can to help out :)