-
Notifications
You must be signed in to change notification settings - Fork 41
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
Adjustments to test with python 3.12 #122
Conversation
@benhoyt I happened to find these unit tests failures when running the unit tests on python 3.12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 on pyfakefs update. I vaguely remember having to do that in the past in some other repo.
mock_output.assert_called_once_with( | ||
["sysctl", "-n", "vm.swappiness", "other_value"], stderr=-2, universal_newlines=True | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old test was just bad, wasn't it?
Meaning that the 2 settings were fetched serially before, and in a single command now. The code change must've been done without correcting the tests at some point prior, mustn't it?
I wonder if stderr=-2
is actually significant, but then while the value could be checked against ANY
, the presence of the kwarg cannot be easily ignored, which is a bit of an impasse.
Happy to have this change regardless :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for these changes. Mind taking a look the failing integration test, too?
Perhaps the old version needs more time to stop? |
Aye: looks like refreshing the lxd snap fails if we're in a big hurry?
|
there's also this in the pytest logs:
looks like something has gone wrong with snapd on the machine during refresh that it cannot communicate with lxd?? |
Ah yes, I just noticed that too @addyess. Weird, it's like the snap daemon isn't running or something. We'll let you off the hook. ;-) I'll merge this PR and we can address the integration test failures separately. |
Opened #123 to track fixing the integration test. |
This...time...only... |
While testing with python3.12, i had to bump
pyfakefs==5.5.0
due to some changes inpathlib
After this adjustment, i found bad tests in
test_sysctl
.instead of
swapped to
Also, it seems codespell now doesn't like
assertIn
so let's start ignoring reasonable words