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 failing test_net_if_addrs when some network interfaces are disconnected #895

Merged
merged 1 commit into from
Sep 30, 2016

Conversation

embray
Copy link
Contributor

@embray embray commented Sep 27, 2016

Trying to bind a socket to the address of an interface that's down can fail, so skip in that case.

@embray
Copy link
Contributor Author

embray commented Sep 30, 2016

A couple tests failed on AppVeyor due to the issue fixed by #897

@giampaolo
Copy link
Owner

Can you point me to the failure or provide the traceback? Even if the interface is down the IP addresses should be consistent. If the failure is because the address cannot be binded it's ok (so we skip that), but if the test fails because the address is not a valid IPv4 address or something then we want a failure.

@embray
Copy link
Contributor Author

embray commented Sep 30, 2016

Ah, to be clear it's simply that the address can't be binded. The error this is fixing look like this:

======================================================================
ERROR: test_net_if_addrs (test_system.TestSystemAPIs)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\Erik M. Bray\src\psutil-win\psutil\tests\test_system.py", line 574, in test_net_if_addrs
    s.bind((addr.address, 0))
OSError: [WinError 10049] The requested address is not valid in its context

I can reproduce this very simply outside psutil as well, simply by looping over the IP addresses of the interfaces and trying to bind a socket to them. It fails with this error only on interfaces that are down.

@giampaolo
Copy link
Owner

What the address looks like? Is it valid?

On Sep 30, 2016 1:58 PM, "Erik Bray" notifications@github.com wrote:

Ah, to be clear it's simply that the address can't be binded. The error
this is fixing look like this:

ERROR: test_net_if_addrs (test_system.TestSystemAPIs)

Traceback (most recent call last):
File "C:\Users\Erik M. Bray\src\psutil-win\psutil\tests\test_system.py", line 574, in test_net_if_addrs
s.bind((addr.address, 0))
OSError: [WinError 10049] The requested address is not valid in its context

I can reproduce this very simply outside psutil as well, simply by looping
over the IP addresses of the interfaces and trying to bind a socket to
them. It fails with this error only on interfaces that are down.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#895 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAplLF5NpjkPN3G9Emcdukh93hcJ8amtks5qvPljgaJpZM4KHnV-
.

@embray
Copy link
Contributor Author

embray commented Sep 30, 2016

They have link-local addresses. For example:

'Local Area Connection* 2': [
    snic(family=<AddressFamily.AF_LINK: -1>, address='90-2E-1C-47-65-7C', netmask=None, broadcast=None, ptp=None),
    snic(family=<AddressFamily.AF_INET: 2>, address='169.254.28.236', netmask='255.255.0.0', broadcast=None, ptp=None),
    snic(family=<AddressFamily.AF_INET6: 23>, address='fe80::d9b9:5207:4c85:1cec', netmask=None, broadcast=None, ptp=None)
]

@giampaolo
Copy link
Owner

It probably is enough to just skip the bind() part if the interface is down - no?

@embray
Copy link
Contributor Author

embray commented Sep 30, 2016

I think I see what you're saying, but if it just skipped the bind() all that would be left would be initializing a socket with addr.family. That doesn't seem worth testing since before that point it already asserts that addr.family is a valid, known value.

@embray
Copy link
Contributor Author

embray commented Sep 30, 2016

In other words, all this PR changes is skipping the part of the test that creates a socket and binds the address to it. It doesn't skip any other assertions. If it skips the bind there's not much point in creating the socket to bind to.

@giampaolo
Copy link
Owner

I erroneously thought we were also checking address validity but we're not. Sorry about that. Merging.

@giampaolo giampaolo merged commit ce0b473 into giampaolo:master Sep 30, 2016
@embray
Copy link
Contributor Author

embray commented Sep 30, 2016

Ok, no problem. Thanks!

@embray embray deleted the bug/test_net_if_addrs branch September 30, 2016 13:36
@giampaolo
Copy link
Owner

Thank you dude. ;)

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

2 participants