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: TestInterfacesWithNetsh and TestInterfaceHardwareAddrWithGetmac fail #13606

Closed
alexbrainman opened this issue Dec 14, 2015 · 9 comments

Comments

Projects
None yet
4 participants
@alexbrainman
Copy link
Member

commented Dec 14, 2015

This

c:\dev\go\src\net>go test -short net
--- FAIL: TestInterfacesWithNetsh (0.34s)
net_windows_test.go:253: unexpected interface list ["Local Area Connection" "Loopback Pseudo-Interface 1" "isatap.sge.local"], want ["Local Area Connection" "Loopback Pseudo-Interface 1"]
--- FAIL: TestInterfaceHardwareAddrWithGetmac (0.67s)
net_windows_test.go:449: unexpected MAC addresses ["Local Area Connection=00:82:34:95:45:11" "isatap.sge.local=00:00:00:00:00:00:00:e0"], want ["Local Area Connection=00:82:34:95:45:11"]
FAIL
FAIL net 13.452s

fails on one of my computers. We would have to rearrange tests a little bit.

/cc @mikioh

Alex

@mikioh mikioh added the OS-Windows label Dec 14, 2015

@mikioh mikioh added this to the Go1.6 milestone Dec 14, 2015

@mikioh

This comment has been minimized.

Copy link
Contributor

commented Dec 14, 2015

Calling GetAdaptersAddresses with GAA_FLAG_INCLUDE_ALL_INTERFACES for listing all network interfaces including virtual/logical/pseudo stuff, or just checking the results contained in scraping stuff.

It's all your call. Because unlike BSD variants and Linux, Windows has not only common virtual interfaces such as ISATAP/6to4/Teredo/L2TP/PPPoE tunnel interfaces but pseudo-application helper network interfaces such as IKE blah blah.

@mattn

This comment has been minimized.

Copy link
Member

commented Dec 14, 2015

FYI, GetAdaptersAddresses is not provided wide function. So it need to convert character-set using MultiByteToWideChar.

@alexbrainman

This comment has been minimized.

Copy link
Member Author

commented Dec 15, 2015

@mikioh I have plan for fixing TestInterfacesWithNetsh (CL 17850), but looking for suggestions on how to modify TestInterfaceHardwareAddrWithGetmac to make it PASS here. Thank you.

Alex

@gopherbot

This comment has been minimized.

Copy link

commented Dec 15, 2015

CL https://golang.org/cl/17850 mentions this issue.

alexbrainman added a commit that referenced this issue Dec 17, 2015

net: include both ipv4 and ipv6 netsh output in TestInterfacesWithNetsh
Also include test for interface state (up or down).

Updates #13606

Change-Id: I03538d65525ddd9c2d0254761861c2df7fc5bd5a
Reviewed-on: https://go-review.googlesource.com/17850
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Run-TryBot: Russ Cox <rsc@golang.org>
@alexbrainman

This comment has been minimized.

Copy link
Member Author

commented Dec 17, 2015

@mikioh TestInterfacesWithNetsh is fixed now. But TestInterfaceHardwareAddrWithGetmac is still broken. Do you have any suggestions of changing the test so it PASSes? Thank you.

Alex

@mikioh

This comment has been minimized.

Copy link
Contributor

commented Dec 18, 2015

@alexbrainman,

Sorry, I'm not a windows user and have no windows box. I'd recommend you to drop or disable TestInterfaceHardwareAddrWithGetmac unless you are sure getmac command uses information from where and how. It's fine because they are auxiliary test cases and we already have other test cases in interface_test.go and interface_windows_test.go.

@mikioh mikioh added the Testing label Dec 18, 2015

@alexbrainman

This comment has been minimized.

Copy link
Member Author

commented Dec 19, 2015

... It's fine because they are auxiliary test cases and we already have other test cases in interface_test.go and interface_windows_test.go.

I have created CL 17993 to test your theory - just to see what tests will fail if I leave Interface.HardwareAddr blank. It appears (https://storage.googleapis.com/go-build-log/74b9c8ae/windows-amd64-gce_e2dcba01.log) only TestInterfaceHardwareAddrWithGetmac fails:

--- FAIL: TestInterfaceHardwareAddrWithGetmac (1.13s)
    net_windows_test.go:515: unexpected MAC addresses ["Local Area Connection=00:00:00:00:00:00"], want ["Local Area Connection=42:01:0a:f0:00:13"]
FAIL
FAIL    net 25.539s

so if we disable this test, then we can easily regress just like I did in CL 17993.

I think we need to keep as much of TestInterfaceHardwareAddrWithGetmac as we can.

Alex

@mikioh

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2015

I have created CL 17993 to test your theory

Sorry, I don't understand why it's related to. If you want to have a unit test which verifies payload for some reason, please do it. I don't think TestInterfaceHardwareAddrWithGetmac is a unit or integration test case. It looks an end-to-end test case. For standard library, there's no reason to prefer end-to-end test to unit/integration test.

@gopherbot

This comment has been minimized.

Copy link

commented Dec 19, 2015

CL https://golang.org/cl/17994 mentions this issue.

@golang golang locked and limited conversation to collaborators Dec 29, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.