Hermetic DNS/HTTP tests + resolve4/6 wire-protocol migration (#495)#869
Merged
Conversation
…ind a trait A handful of tests made real outbound DNS/HTTP calls. On a flaky CI runner the in-flight request keeps the event loop alive to the 30s test timeout (false-red), and the matrix's default fail-fast then cancels the other platforms. A prior Skip.If(output.Length==0) band-aid masked genuine no-output regressions. - FetchReturnsPromise: hit the local MockHttpServer instead of example.com. - DnsRecordTypeTests: delete the two *_InvalidDomain_* tests (already covered deterministically by DnsFakeServerModuleTests NXDOMAIN); tag the google.com LiveSmoke tests [Trait Category=LiveNetwork]; drop the Skip-on-empty guard. - DnsModuleTests: tag the two invalid-hostname lookup tests LiveNetwork (dns.lookup uses getaddrinfo - no fake-server seam). - LiveNetworkHosts.cs (new): the single sanctioned home for external host literals. - LiveNetworkHermeticityTests.cs (new): guardrail meta-test - fails the build if an inline external literal appears outside that file, or a file using it isn't tagged. - CI: strategy.fail-fast: false; Test step --filter "Category!=LiveNetwork". - DnsFakeServerModuleTests: answer A/AAAA from the fake server (used by the resolve4 migration in the next commit). Refs #495
…semantics)
resolve4/resolve6 (and resolve() with the default/A/AAAA rrtype) previously used
the OS resolver (Dns.GetHostEntry / getaddrinfo), which reads the hosts file and
has no SHARPTS_DNS_SERVER redirect seam. That made them non-hermetic in tests and
diverged from Node, where resolve* uses c-ares (querying the configured DNS server,
not the hosts file). They now go through the existing emitted DNS wire protocol.
Behavior change: resolve4('localhost') no longer returns 127.0.0.1 from the hosts
file — it queries the DNS server and typically returns ENOTFOUND, matching Node.
Use dns.lookup for hosts-file resolution (unchanged — still getaddrinfo). reverse,
lookup, and lookupService also stay on getaddrinfo (Node parity).
- Interpreter: DnsRecordResolver.ResolveA/ResolveAaaa and the custom-server A/AAAA
cases route to DnsWireProtocol.Query; DnsModuleInterpreter.ResolveAddresses too.
Widened the resolve4/resolve6 callback catch from SocketException to Exception
(the wire protocol throws Exception("Runtime Error: dns.x ECODE host");
ExtractErrorCode already parses both shapes).
- Compiled: the three A/AAAA emit sites in RuntimeEmitter.Dns.cs (DnsResolveRecord,
DnsWrapper_resolve4/6, DnsWrapper_resolve) route through the emitted wire protocol
(DnsDoQuery); removed the now-dead EmitDnsResolveAddresses helper. No new
SharpTS.dll late-binding — DnsResolveRecord is already pure emitted IL.
- Tests: DnsAsyncTests + DnsResolverTests resolve4/resolve tests now run against a
loopback FakeDnsServer (SHARPTS_DNS_SERVER / setServers) with exact-value
assertions, in both runtimes. dns.lookup/reverse tests stay on loopback.
- Docs: note the resolve* vs lookup distinction (node-modules-api.md, STATUS-NODE.md).
Refs #495
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.
Closes #495.
Problem
A few tests made real outbound DNS/HTTP calls. On a flaky CI runner the in-flight request keeps the event loop alive (the #320 mechanism) until the 30s test timeout → false-red; and the matrix's default
fail-fastthen cancels the other two platforms (1 flake = 3 red jobs). A priorSkip.If(output.Length==0)band-aid masked genuine no-output regressions and didn't even catch the real hang mode.Goal: every test is either fully hermetic or explicitly tagged
LiveNetworkand excluded from CI — enforced by a guardrail meta-test — and theSkipband-aid is gone.Two commits: A (test-infra, no runtime change) and B (a Node/c-ares-accurate runtime semantics change).
Commit A — hermetic test infrastructure
FetchReturnsPromise→ localMockHttpServerinstead ofexample.com.DnsRecordTypeTests.*_InvalidDomain_*(already covered deterministically byDnsFakeServerModuleTestsNXDOMAIN tests).google.comLiveSmoke + invalid-hostnamelookup)[Trait("Category","LiveNetwork")]; removed theSkip.If(output.Length==0)guards.Infrastructure/LiveNetworkHosts.cs— the single sanctioned home for external host literals.CompilerTests/LiveNetworkHermeticityTests.csguardrail (modeled onStandaloneDllTests): fails the build if an inline external-network literal (a non-loopbackfetch('http…'),google.com, or a reserved*.examplename) appears outside that file, and that any file using aLiveNetworkHostsconstant isLiveNetwork-tagged. Deliberately keyed on the network-call pattern, not bareexample.com(which has ~120 legit hermetic uses: URL parsing,Request/Response,Agent.getName, loopback DNS tests).strategy.fail-fast: false+dotnet test --filter "Category!=LiveNetwork".Commit B —
resolve4/resolve6→ DNS wire protocolresolve4/resolve6(andresolve()with the default/A/AAAA rrtype) used the OS resolver (Dns.GetHostEntry/getaddrinfo), which reads the hosts file and has noSHARPTS_DNS_SERVERredirect seam — making them non-hermetic and divergent from Node (whereresolve*uses c-ares). They now route through the existing emitted DNS wire protocol (the A/AAAA parse already existed in both modes).DnsRecordResolver,DnsModuleInterpreter.ResolveAddresses) + the three compiled emit sites inRuntimeEmitter.Dns.cs; removed the now-deadEmitDnsResolveAddresses. No new SharpTS.dll late-binding (the wire-protocol path is already pure emitted IL).resolve4/resolve6callbackcatchfromSocketExceptiontoException(the wire protocol throwsException("Runtime Error: dns.x ECODE host")).reverse/lookup/lookupServiceintentionally stay on getaddrinfo (Node parity).resolve4/resolve6now run against a loopbackFakeDnsServer(SHARPTS_DNS_SERVER/setServers) with exact-value assertions, both runtimes.Behavior change (documented in
docs/node-modules-api.md+STATUS-NODE.md):resolve4('localhost')now returnsENOTFOUNDrather than127.0.0.1— usedns.lookupfor hosts-file resolution. Matches Node.Verification
fetch('https://google.com'), green on revert; confirmed it does not flag the existing legitexample.comlines.LiveNetworkand excluded by--filter "Category!=LiveNetwork".Follow-up (out of scope, tracked separately)
A fail-fast timeout on the remaining getaddrinfo path (
lookup/lookupService/reverse) — after this PR there's no getaddrinfo hang surface left in CI, so it's deferred as a product-robustness hardening.