fix(tests): use .invalid TLD for unresolvable DNS test#12416
fix(tests): use .invalid TLD for unresolvable DNS test#12416Dreamsorcerer merged 3 commits intoaio-libs:masterfrom
Conversation
test_aiohttp_request_ctx_manager_not_found was flaky on networks where 'wrong-dns-name.com' could occasionally resolve (e.g. ISP NXDOMAIN hijacking or transient parking entries), causing the request to proceed and trip the inner assertion. Switch to the reserved .invalid TLD (RFC 2606), which is guaranteed never to resolve.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #12416 +/- ##
==========================================
- Coverage 98.92% 98.92% -0.01%
==========================================
Files 134 134
Lines 46751 46750 -1
Branches 2430 2429 -1
==========================================
- Hits 46249 46248 -1
Misses 373 373
Partials 129 129
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Merging this PR will not alter performance
Comparing Footnotes
|
webknjaz
left a comment
There was a problem hiding this comment.
Please fill out the PR template.
- CHANGES/11293.misc.rst: drop copilot credit, switch RFC link to :rfc: role - tests/test_client_functional.py: revert verbose comment & assertion message Per webknjaz review on aio-libs#12416 Signed-off-by: Bojun Chai <bojunchai@microsoft.com>
|
Thanks for the review @webknjaz — pushed e127668 addressing all four notes:
Let me know if you'd like the PR description filled out against the template as well. |
|
@webknjaz thanks — PR description filled out against the template. The change itself is just the hostname swap to |
Backport to 3.14: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 5503528 on top of patchback/backports/3.14/5503528282ee13a2fcdad13b641e38c3a18eaf81/pr-12416 Backporting merged PR #12416 into master
🤖 @patchback |
Closes #11293
What do these changes do?
Switch the unresolvable-DNS test hostname from
wrong-dns-name.comtowrong-dns-name.invalid, using the reserved.invalidTLD (RFC 2606), so the test cannot be flaked by NXDOMAIN hijacking, parking pages, or cached records on networks that opportunistically resolve.comtypos.Also added a
"never executed"message to the innerassert Falseto match the sibling test and aid debugging when something does enter the body.Are there changes in behavior for the user?
No — test-only change. No public API or runtime behavior is affected.
Is it a substantial burden for the maintainers to support this?
No — single hostname swap in one test, no new dependencies, and
.invalidis guaranteed unresolvable by RFC 2606, so there is no new infrastructure to maintain.Related issue number
Fixes #11293
Root cause
test_aiohttp_request_ctx_manager_not_foundrelied onhttp://wrong-dns-name.comfailing DNS resolution, but on some networks (notably macOS / certain ISPs) that domain can intermittently resolve. When it resolves, the request proceeds past theasync with, the innerassert Falsetrips, and the outerpytest.raises(ClientConnectionError)fails.Regression test
tests/test_client_functional.py::test_aiohttp_request_ctx_manager_not_found— same test, now deterministic; it asserts that entering anaiohttp.requestcontext manager for an unresolvable host raisesClientConnectionErrorand never enters the body.Verification
Manually verified via
socket.getaddrinfo('wrong-dns-name.invalid', 80)raisesgaierror, confirming.invalidis unresolvable and the test will reliably hit theClientConnectionErrorpath. Local pytest env was missing some required plugins (textual), so the in-tree run is left to CI.Checklist
CONTRIBUTORS.txtCHANGES/folder (CHANGES/11293.misc.rstis included in this PR)