Initial rpmbuld failure - test_disable_ip_forwarding #2717
Assignees
Comments
|
Apologies for failing with these tests... It seems I missed some mocking somewhere. Here's what I noticed: I ran the test once first and it all passed, even after stopping, uninstall tailscale and its repo (and after a reboot): I noticed, however, that I had the file Now, if I manually delete that file and run the tests again, I get the same failure you have: The devil is in the details here: the order in which the tests are run: If we run again these tests, they all pass now given the file is present: |
|
@FroggyFlox Thanks for clearing that one up. And my apologies for missing this initially in my review. Bit by bit. |
|
To prevent actually writing def test_enable_ip_forwarding(self):
"""enable_ip_forwarding() calls write with the correct contents
Enable_ip_forwarding should write to /etc/sysctl.d/99-tailscale.conf
with the following lines
- net.ipv4.ip_forward = 1
- net.ipv6.conf.all.forwarding = 1
"""
priority = 99
name = "tailscale"
file_path = f"{SYSCTL_CONFD_PATH}{priority}-{name}.conf"
m = mock_open()
with patch("system.network.open", m):
enable_ip_forwarding(name=name, priority=priority)
# Test that the file was opened in 'w' mode
m.assert_called_once_with(file_path, "w")
# Test that 'write' was called with the correct content
calls = [
call("net.ipv4.ip_forward = 1\n"),
call("net.ipv6.conf.all.forwarding = 1\n"),
]
m().write.assert_has_calls(calls, any_order=False)
# Test that run_command was called as expected
self.mock_run_command.assert_called_once_with(
[SYSCTL, "-p", file_path], log=True
)With regards to testing |
|
Of course, it is right after I write this that I finally understood how it works. Following some sparse online answers on how to do that, it appears we can manually re-create the class PseudoDirEntry:
def __init__(self, name, path, is_dir, stat):
self.name = name
self.path = path
self._is_dir = is_dir
self._stat = stat
def is_dir(self):
return self._is_dir
def stat(self):
return self._stat... and use it as a return value for our def test_disable_ip_forwarding(self):
"""test proper call of os.remove() and final sysctl command"""
# mock os.scandir()
self.patch_os_scandir = patch("system.network.os.scandir")
self.mock_os_scandir = self.patch_os_scandir.start()
self.mock_os_scandir.return_value.__enter__.return_value = [
MockDirEntry(name="99-tailscale.conf", path="/etc/sysctl.d/99-tailscale.conf", is_file=True),
MockDirEntry(name="70-yast.conf", path="/etc/sysctl.d/70-yast.conf", is_file=True),
]
# mock os.remove()
self.patch_os_remove = patch("system.network.os.remove")
self.mock_os_remove = self.patch_os_remove.start()
# mock run_command()
self.mock_run_command.return_value = [""], [""], 0
disable_ip_forwarding(name="tailscale")
# test os.remove() was called
self.mock_os_remove.assert_called_once_with("/etc/sysctl.d/99-tailscale.conf")With this, all tests pass even without |
FroggyFlox
added a commit
to FroggyFlox/rockstor-core
that referenced
this issue
Oct 19, 2023
We had two mocking deficiencies: - one that resulted in one test actually writing a file to disk - one failing to properly mock os.scandir (we were thus depending on the real system state) This commit fixes the former by combining 2 previously split tests, thereby sharing the same mock (we're not writing to disk anymore). To fix the latter, we instantiate a new class used to mock os.scandir return values that we can now properly control.
FroggyFlox
added a commit
to FroggyFlox/rockstor-core
that referenced
this issue
Oct 19, 2023
We had two mocking deficiencies: - one that resulted in one test actually writing a file to disk - one failing to properly mock os.scandir (we were thus depending on the real system state) This commit fixes the former by combining 2 previously split tests, thereby sharing the same mock (we're not writing to disk anymore). To fix the latter, we instantiate a new class used to mock os.scandir return values that we can now properly control.
phillxnet
added a commit
that referenced
this issue
Oct 20, 2023
…e---test_disable_ip_forwarding Fix mocking insufficiencies in system.network.py #2717
|
Closing as fixed by #2718 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
During the initial review of #2712 (comment #2712 (review)) there was an as yet unexplained fail in a new test regarding ip forwarding:
On further fresh rpmbuild attempts (other OS/arch targets) this has been seen again on all systems that have not previously done an rpmbuild with this test included. We likely have some inadvertent system sensitivity/interaction here.
The text was updated successfully, but these errors were encountered: