connectd/tests: fix test_announce_and_connect_via_dns on macOS#9143
connectd/tests: fix test_announce_and_connect_via_dns on macOS#9143nGoline wants to merge 1 commit into
Conversation
89d3d46 to
b7b271c
Compare
| socket.getaddrinfo('localhost.localdomain', 12345, 0, socket.SOCK_STREAM) | ||
| except socket.gaierror: | ||
| pytest.skip("localhost.localdomain not resolvable; add '127.0.0.1 localhost.localdomain' to /etc/hosts") | ||
|
|
There was a problem hiding this comment.
Instead of skipping the test, I think we might be able to use "test.localhost," as mac allows subdomains on localhost. If that doesn't work I wonder if just setting it to "localhost" would satisfy what the test is trying to do.
If none of those work and we have to skip the test -- we should skip the test with a clause for Mac, probably something like:
@unittest.skipIf(sys.platform == "darwin")Doing it explicitly this way will make it easy to keep track of which tests we're skipping
There was a problem hiding this comment.
This actually skips with a message for the dev, asking them to add the host to /etc/hosts.
I can try the other options to see if something goes through...
There was a problem hiding this comment.
I dug into the two options you proposed and hit a wall with test.localhost. Here's what I found:
test.localhost resolves correctly on macOS via mDNSResponder (no /etc/hosts entry needed), but it does not resolve on Ubuntu 24.04 containers used on CI, which lack systemd0resolved. To work around that I tried a platform-specific approach: test.localhost on macOS, localhost.localdomain on Linux, and that passed on Linux. However, the macOS teardown fails with a BROKEN log because mDNSResponder opens temporary sockets internally during the resolution independently of AI_ADDRCONFIG. I've tried guarding connectd using AI_ADDRCONFIG with #ifndef __APPLE__, but that's not the source of the sockets here, it's the mDNSResponder daemon itself. Any hostname that goes through mDNSResponder on macOS will trigger this.
localhost was not tried because the test specifically avoids it. dev-allow-localhost would let nodes connect to 127.0.0.1 without DNS, defeating the purpose of the test.
That leaves us with two honest options:
- Explicit
@unittest.skipIf(sys.platform == "darwin")(your fallback suggestion): keeplocalhost.localdomain, which resolves cleanly on Linux and avoidsmDNSResponderentirely. Skip on macOS with a clear, trackable decorator. - Fix
dev_report_fdsto toleratemDNSRespondersockets: teachconnectd'sfd-leakchecker that sockets opened by the system resolver are not leaks. This would lettest.localhostwork cleanly on macOS, but it's a non-trivial change to theioframework and probably belongs in a separate PR.
I've gone with option 1 for this PR and will file a follow-up for option 2.
localhost.localdomain is not present in /etc/hosts on macOS by default, so the test's DNS-resolve step fails immediately. Any hostname that does resolve on macOS (e.g. test.localhost via mDNSResponder) causes mDNSResponder to open temporary sockets which dev_report_fds() flags as unowned (BROKEN) at teardown. Skip the test explicitly on macOS with @unittest.skipIf so the reason is visible and trackable. A follow-up can fix dev_report_fds to tolerate resolver sockets and re-enable the test on macOS. Changelog-None
b7b271c to
e47a833
Compare
Summary
test_announce_and_connect_via_dnswith a clear message whenlocalhost.localdomainis not resolvable (macOS omits it from/etc/hostsby default)AI_ADDRCONFIGintry_connect_one_addr()with#ifndef __APPLE__: on macOS/BSD,AI_ADDRCONFIGopens temporary probe sockets that are not tracked by theioframework, causingdev_report_fds()to report them as unowned (**BROKEN**); Linux keeps the optimization unchangedCloses #9012
Test plan
uv run pytest tests/test_gossip.py::test_announce_and_connect_via_dns -vpasses on Linuxlocalhost.localdomainin/etc/hosts: test is skipped with a descriptive messagelocalhost.localdomainin/etc/hosts: no**BROKEN**fd messages from connectd at teardown🤖 Generated with Claude Code