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

Fixes pr #87

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Fixes pr #87

wants to merge 8 commits into from

Conversation

lanrat
Copy link
Contributor

@lanrat lanrat commented Apr 26, 2021

@grandcat
Copy link
Owner

Thanks for your contribution.
However, Windows test is failing since this change.

@lanrat
Copy link
Contributor Author

lanrat commented Apr 26, 2021

Thanks for checking out this PR.

The Windows test that is failing is below:

$ go test --parallel=1
go: finding github.com/pkg/errors v0.9.1
2021/04/26 18:25:21 [zeroconf] no suitable IPv4 interface: not implemented on windows/amd64
2021/04/26 18:25:21 [zeroconf] no suitable IPv6 interface: not implemented on windows/amd64
panic: while registering mdns service: no supported interface
goroutine 8 [running]:
github.com/grandcat/zeroconf.startMDNS(0x680e00, 0xc0000461e0, 0x22b8, 0x6465a8, 0x12, 0x6455a7, 0x10, 0x642786, 0x6)
	C:/Users/travis/gopath/src/github.com/grandcat/zeroconf/service_test.go:25 +0x323
created by github.com/grandcat/zeroconf.TestBasic
	C:/Users/travis/gopath/src/github.com/grandcat/zeroconf/service_test.go:43 +0x123
exit status 2
FAIL	github.com/grandcat/zeroconf	0.080s
The command "go test --parallel=1" exited with 1.

The test is failing for Windows in this PR because of the additional error checking. Previously the test would fail silently because the errors were not checked. The reason for the failure was not introduced in this PR, it was just made visible.

The test can be updated to ignore the error on Windows, or fixed to better support Windows. Both were initially out of scope for this PR.

@grandcat what are your thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

server.Shutdown() should print errors
2 participants