gh-156031: Restore urllib.request._opener in test_httpservers - #156032
Open
ayaangazali wants to merge 2 commits into
Open
gh-156031: Restore urllib.request._opener in test_httpservers#156032ayaangazali wants to merge 2 commits into
ayaangazali wants to merge 2 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 |
aisk
reviewed
Aug 19, 2026
Member
There was a problem hiding this comment.
Test only changes don't requires a news entry file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 :)