Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build-and-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
runs-on: ubuntu-22.04
strategy:
matrix:
python-version: ["3.8", "3.10"]
python-version: ["3.8", "3.10", "3.12"]
steps:
- name: Checkout
uses: actions/checkout@v3
Expand Down
23 changes: 15 additions & 8 deletions tests/unit/test_sysctl.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,9 @@ def test_create_snapshot(self, mock_load, mock_output):
config._desired_config = {"vm.swappiness": "0", "other_value": "10"}
snapshot = config._create_snapshot()

assert mock_output.called_with(["sysctl", "vm.swappiness", "-n"])
assert mock_output.called_with(["sysctl", "other_value", "-n"])
mock_output.assert_called_once_with(
["sysctl", "-n", "vm.swappiness", "other_value"], stderr=-2, universal_newlines=True
)
Comment on lines +211 to +213
Copy link
Contributor

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 :)

assert snapshot == {"vm.swappiness": "1", "other_value": "5"}

@patch("charms.operator_libs_linux.v0.sysctl.check_output")
Expand All @@ -222,7 +223,9 @@ def test_restore_snapshot(self, mock_load, mock_output):
snapshot = {"vm.swappiness": "1", "other_value": "5"}
config._restore_snapshot(snapshot)

assert mock_output.called_with(["sysctl", "vm.swappiness=1", "other_value=5"])
mock_output.assert_called_once_with(
["sysctl", "vm.swappiness=1", "other_value=5"], stderr=-2, universal_newlines=True
)

@patch("charms.operator_libs_linux.v0.sysctl.check_output")
@patch("charms.operator_libs_linux.v0.sysctl.Config._load_data")
Expand All @@ -233,7 +236,9 @@ def test_syctl(self, mock_load, mock_output):

result = config._sysctl(["-n", "vm.swappiness"])

assert mock_output.called_with(["sysctl", "-n", "vm.swappiness"])
mock_output.assert_called_once_with(
["sysctl", "-n", "vm.swappiness"], stderr=-2, universal_newlines=True
)
assert result == ["1"]

@patch("charms.operator_libs_linux.v0.sysctl.check_output")
Expand All @@ -246,7 +251,9 @@ def test_syctl_error(self, mock_load, mock_output):
with self.assertRaises(sysctl.CommandError) as e:
config._sysctl(["exception"])

assert mock_output.called_with(["sysctl", "exception"])
mock_output.assert_called_once_with(
["sysctl", "exception"], stderr=-2, universal_newlines=True
)
assert e.exception.message == "Error executing '['sysctl', 'exception']': error on command"

@patch("charms.operator_libs_linux.v0.sysctl.Config._sysctl")
Expand All @@ -259,7 +266,7 @@ def test_apply_without_failed_values(self, mock_load, mock_sysctl):
config._desired_config = {"vm.swappiness": "0"}
config._apply()

assert mock_sysctl.called_with(["vm.swappiness=0"])
mock_sysctl.assert_called_with(["vm.swappiness=0"])

@patch("charms.operator_libs_linux.v0.sysctl.Config._sysctl")
@patch("charms.operator_libs_linux.v0.sysctl.Config._load_data")
Expand All @@ -272,7 +279,7 @@ def test_apply_with_failed_values(self, mock_load, mock_sysctl):
with self.assertRaises(sysctl.ApplyError) as e:
config._apply()

assert mock_sysctl.called_with(["vm.swappiness=0"])
mock_sysctl.assert_called_with(["vm.swappiness=0"])
self.assertEqual(e.exception.message, "Unable to set params: ['vm.swappiness']")

@patch("charms.operator_libs_linux.v0.sysctl.Config._sysctl")
Expand All @@ -286,7 +293,7 @@ def test_apply_with_partial_failed_values(self, mock_load, mock_sysctl):
with self.assertRaises(sysctl.ApplyError) as e:
config._apply()

assert mock_sysctl.called_with(["vm.swappiness=0", "net.ipv4.tcp_max_syn_backlog=4096"])
mock_sysctl.assert_called_with(["vm.swappiness=0", "net.ipv4.tcp_max_syn_backlog=4096"])
self.assertEqual(e.exception.message, "Unable to set params: ['vm.swappiness']")

@patch("charms.operator_libs_linux.v0.sysctl.Config._load_data")
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ deps =
pytest
coverage[toml]
-r{toxinidir}/requirements.txt
pyfakefs==4.4.0
pyfakefs==5.5.0
dbus-fast==1.90.2
commands =
coverage run --source={[vars]lib_dir} \
Expand Down