libnetwork: rewrite some tests to use gotest.tools#49329
libnetwork: rewrite some tests to use gotest.tools#49329thaJeztah merged 5 commits intomoby:masterfrom
Conversation
thaJeztah
added
status/2-code-review
area/networking
This comment was marked as resolved.
This comment was marked as resolved.
thaJeztah
force-pushed
the
libnetwork_use_errdefs_step2
branch
from
4791392 to
952f7d4
Compare
| if iface.AddressIPv6() != nil { | ||
| assert.Check(t, iface.AddressIPv6().IP != nil, "Invalid IPv6 address returned: %v", iface.AddressIPv6()) | ||
| } |
There was a problem hiding this comment.
The old code did the same - but could align this with the IPv4 check, and make sure it is IPv6? ...
| if iface.AddressIPv6() != nil { | |
| assert.Check(t, iface.AddressIPv6().IP != nil, "Invalid IPv6 address returned: %v", iface.AddressIPv6()) | |
| } | |
| if iface.AddressIPv6() != nil { | |
| assert.Check(t, iface.AddressIPv6().IP.To6() != nil, "Invalid IPv6 address returned: %v", iface.AddressIPv6()) | |
| } |
There was a problem hiding this comment.
Hm.. there's no To6(). There' a To16(), but looking at that one again, it would be happy to return both IPv4 or IPv6 https://github.com/golang/go/blob/608acff8479640b00c85371d91280b64f5ec9594/src/net/ip.go#L226-L236
So I guess we need to do a To4() and check that it's nil (i.e., "not and IPv4 address, so IPv6")
First step; this is just a find and replace Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Prefering Check here over NilError so that all defers wil be executed, instead of potentially failing on the first one. Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
libnetwork_use_errdefs_step2
branch
from
952f7d4 to
2a8fe5e
Compare
Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
libnetwork_use_errdefs_step2
branch
from
2a8fe5e to
203d653
Compare
|
Flaky test? (was updated in #49150 with some retries) |
|
Still LGTM.
Hmm, that's worrying. I don't think that update added any retries, it added a live-restore test case. But, I guess this means there's some race between interfaces coming up or being ready and sending the unsolicited NAs. |
|
Let's keep an eye if we see it fail more often; my comment above should help find it back. It could still be a one-off! |
Yes, I've been running the interesting part of test in a tight loop and haven't seen a failure. But perhaps it's a platform specific thing. I'll carry on investigating for a bit. |
2 participants
This does not yet include changes for error-assertions; pushing those separate for closer review
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)