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

net: some tests are skipped on Windows due to os.Getuid() always returns -1 #40886

Open
zhangyoufu opened this issue Aug 19, 2020 · 5 comments
Open

Comments

@zhangyoufu
Copy link

@zhangyoufu zhangyoufu commented Aug 19, 2020

Does this issue reproduce with the latest release?

Yes

What did you do?

go test -v net

What did you expect to see?

Run all tests that are applicable.

What did you see instead?

=== RUN   TestIPConnLocalName
--- PASS: TestIPConnLocalName (0.00s)
    iprawsock_test.go:93: skipping ip4:icmp test
    iprawsock_test.go:93: skipping ip4:icmp test
    iprawsock_test.go:93: skipping ip4:icmp test
=== RUN   TestIPConnRemoteName
--- SKIP: TestIPConnRemoteName (0.00s)
    iprawsock_test.go:109: ip:tcp test
=== RUN   TestIPv6MulticastListener
--- SKIP: TestIPv6MulticastListener (0.00s)
    listen_test.go:619: must be root

Related code

// testableNetwork reports whether network is testable on the current
// platform configuration.
func testableNetwork(network string) bool {
ss := strings.Split(network, ":")
switch ss[0] {
case "ip+nopriv":
case "ip", "ip4", "ip6":
switch runtime.GOOS {
case "plan9":
return false
default:
if os.Getuid() != 0 {
return false
}
}

go/src/net/listen_test.go

Lines 603 to 620 in d3a411b

// TestIPv6MulticastListener tests both single and double listen to a
// test listener with same address family, same group address and same
// port.
func TestIPv6MulticastListener(t *testing.T) {
testenv.MustHaveExternalNetwork(t)
switch runtime.GOOS {
case "plan9":
t.Skipf("not supported on %s", runtime.GOOS)
case "solaris", "illumos":
t.Skipf("not supported on solaris or illumos, see issue 7399")
}
if !supportsIPv6() {
t.Skip("IPv6 is not supported")
}
if os.Getuid() != 0 {
t.Skip("must be root")
}

@networkimprov
Copy link

@networkimprov networkimprov commented Aug 20, 2020

@gopherbot add OS-Windows

cc @alexbrainman

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Aug 22, 2020

@zhangyoufu that is unfortunately how things are: os.Getuid is not implemented on Windows, and we have to skip tests that require os.Getuid to work. We all know that. Why did you create this issue? Do you propose to fix os.Getuid? Do you propose something else?

Thank you.

Alex

@zhangyoufu
Copy link
Author

@zhangyoufu zhangyoufu commented Aug 22, 2020

Do you propose to fix os.Getuid?

No. I think that uid and sid are totally different. We can't just map SYSTEM/Administrators to uid 0.

Why did you create this issue?

I think that the some test cases can be run on Windows, instead of just skip because os.Getuid is not implemented.

Do you propose something else?

I propose that these test cases should not consult os.Getuid on Windows. We can have helper functions in testenv, judging whether we have enough privilege to perform such test cases.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Aug 22, 2020

I propose that these test cases should not consult os.Getuid on Windows. We can have helper functions in testenv, judging whether we have enough privilege to perform such test cases.

Sounds good to me. But I don't plan to work on it. I will be happy to review someone else's code.

Alex

@cagedmantis cagedmantis added this to the Backlog milestone Aug 24, 2020
@cagedmantis
Copy link
Contributor

@cagedmantis cagedmantis commented Aug 24, 2020

/cc @mikioh

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

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.