Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix socket specs when network not available #12961

Merged

Conversation

straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented Jan 15, 2023

Socket specs were failing when running on a system with no network.

UDP multicast specs are already pretty fragile and handle many points of failure as acceptable because the host system/network might not support UDP multicast. This patch just adds code for another failure type.

Then there are a couple of specs which initiate a DNS lookup to a non-existent domain (doesnotexist.example.org.). When no DNS server is available, getaddrinfo apparently returns EAI_AGAIN to try again later (which of course won't help anything when there's simply no network to reach an upstream DNS server).
I'm not entirely sure if simply accepting EAI_AGAIN is the best way to fix this. The spec could also be marked as pending in that case. But I think success is fine.
Also, I'm wondering if EAI_AGAIN should be handled somewhere in Addrinfo, retrying and finally raise a more specific error.
But that's a whole different story. For now I think these changes should be good to make the spec suite work on isolated machines (a use case for that is the build environment on build.opensuse.org/).

If you want to run specs without network access, I found the easiest way is a docker container with --network none.

@beta-ziliani
Copy link
Member

The Windows job is failing

spec/std/socket/tcp_socket_spec.cr Outdated Show resolved Hide resolved
spec/std/socket/tcp_socket_spec.cr Outdated Show resolved Hide resolved
Co-authored-by: Sijawusz Pur Rahnama <sija@sija.pl>
@beta-ziliani
Copy link
Member

I think marking them as pending instead of success makes more sense.

@straight-shoota
Copy link
Member Author

Yes, that's happening in UDPSocket spec.

The other specs are already expecting failure, they're just picky about the error code. I think accepting EAI_AGAIN as a "successful" failure makes sense. There's no way to tell exactly what caused it, it just indicates "temporary failure". And that should be good for these specs.

@beta-ziliani beta-ziliani added this to the 1.8.0 milestone Jan 19, 2023
@straight-shoota straight-shoota merged commit 54215ce into crystal-lang:master Jan 20, 2023
@straight-shoota straight-shoota deleted the fix/network-specs branch January 20, 2023 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants