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 some test fallout found on Linux/Sparc #1643

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

Conversation

DerDakon
Copy link

This fixes most of the problems reported in #1635.

@DerDakon
Copy link
Author

I don't understand the test error, looks unrelated to me.

Signed-off-by: Rolf Eike Beer <eike@sf-mail.de>
…nd configuration

Signed-off-by: Rolf Eike Beer <eike@sf-mail.de>
@DerDakon
Copy link
Author

DerDakon commented Aug 5, 2021

Ping?

@@ -173,8 +173,10 @@ def test_rlimit(self):
self.assertEqual(hasattr(psutil.Process, "rlimit"), LINUX or FREEBSD)

def test_io_counters(self):
not_on_linux = (LINUX and not os.path.exists('/proc/1/io'))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the meaning of this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's Linux, but the thing doesn't exist here. I'm open for any better name.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. It's OK, let's let it fail.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will constantly fail e.g. on my sparc machine because this simply does not exist there, annoying me during the stabilizations for Gentoo because the testsuite is marked as failed.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are supposed to be "alarms" testing API availability depending on the OS. I prefer to have them failing.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the API is not generally available on Linux. That's all what I'm testing here: if these iocounters are available. Thinking about that this test could result in a false negative for the (unlikely) case that one has locked down /proc access more than usual, e.g. to not show processes of other users.

If you don't like this one just cherry-pick the first one.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the API is not generally available on Linux

It is generally available, at least on the majority of Linux i686 / x86 archs / distros I've seen. Most likely it's not available on Sparc, in which case I prefer to know with an explicit failure.
I understand it is annoying to see a failure on your specific arch / distro, but (in my mind) these tests checking API availability assume psutil is running on the most common arch / distro, not the "uncommon one", and are designed to (fail) be like this on purpose.

If you don't like this one just cherry-pick the first one.

Wouldn't know how to do that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a kernel configuration thing. We have even bug reports that this is failing on amd64 (and again).

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