Skip to content

Commit a94782a

Browse files
bernhardkaindlbernhardkaindl
committed
.net.biosdevname: Fix Popen() using universal_newlines=True, add test
This PR fixes the Popen() in xcp.net.biosdevname like xenserver#38 fixed the Popen of xcp.pci.PCIDevices() using universial_newlines=True, to make the Popen return strings as expected by the code, not bytes. Redirects the call to /sbin/biosdevname to tests/data/biosdevname to provide the test data to tests/test_biosdevname.py:test_nomock() Adds a new test case because the old test case is using the same mock which already failed to find the issue in xenserver#38. Unlike xenserver#38, the new test can't do this by setting PATH="tests/data" because /sbin/biosdevname is an absolute path. To ensure a correct test, the new test avoids mocking Popen() by installing a wrapper for subprocess.Popen() in tests/conftest.py. This needs to be done a global wrapper in pytest in tests/conftest.py because when another test module in the same pytest run imports the testee module first and does not provide the wrapped subprocess.Popen, when importing it, the wrapper for Popen is not installed. The benefit is that this wrapper can be used by all tests for modules which use subprocess.Popen(), and trace all of them from a central place. This can e.g. be used to check the subprocess call done during a testsuite run.
1 parent ae50d08 commit a94782a

File tree

3 files changed

+105
-2
lines changed

3 files changed

+105
-2
lines changed

‎tests/conftest.py

+17-1
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,26 @@
55
This module is run automatically by pytest to define and enable fixtures.
66
"""
77

8+
# pyre does not find the pytest module when run by tox -e py311-pyre
89
# pyre-ignore-all-errors[21]
9-
import pytest # pyre does not find the module when run by tox -e py311-pyre
10+
import subprocess
1011
import warnings
1112

13+
import pytest
14+
15+
# subprocess.Popen() can only be wrapped for all test cases, not individually:
16+
class PopenWrapper(subprocess.Popen):
17+
"""Global wrapper of subprocess.Popen() for pytest tests in this directory"""
18+
def __init__(self, *args, **kwargs):
19+
"""Wrap Popen, replacing /sbin/biosdevname with tests/data/biosdevname"""
20+
# Redirect for tests/test_biosdevname.py: /sbin/biosdevname -> tests/data/biosdevname:
21+
if args[0][0] == "/sbin/biosdevname":
22+
args[0][0] = "tests/data/biosdevname"
23+
super(subprocess.Popen, self).__init__(*args, **kwargs)
24+
25+
subprocess.Popen = PopenWrapper # type: ignore[misc]
26+
"""Wraps subprocess.Popen for test cases which would like to redirect Popen commands"""
27+
1228
@pytest.fixture(autouse=True)
1329
def set_warnings():
1430
"""

‎tests/test_biosdevname.py

+87
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,94 @@ def test_ppn_true(self):
2525
]))
2626

2727
class TestDeviceNames(unittest.TestCase):
28+
expected_devices = {
29+
"eth0": {
30+
"BIOS device": {"physical": "em1", "all_ethN": "eth0"},
31+
"Kernel name": "eth0",
32+
"Permanent MAC": "EC:F4:BB:C3:AF:A8",
33+
"Assigned MAC": "EC:F4:BB:C3:AF:A8",
34+
"Driver": "ixgbe",
35+
"Driver version": "5.9.4",
36+
"Firmware version": "0x8000095c, 19.5.12",
37+
"Bus Info": "0000:01:00.0",
38+
"PCI name": "0000:01:00.0",
39+
"PCI Slot": "embedded",
40+
"SMBIOS Device Type": "Ethernet",
41+
"SMBIOS Instance": "1",
42+
"SMBIOS Label": "Integrated NIC 1",
43+
"sysfs Label": "NIC1",
44+
"Embedded Index": "1",
45+
},
46+
"eth1": {
47+
"BIOS device": {"physical": "em2", "all_ethN": "eth1"},
48+
"Kernel name": "eth1",
49+
"Permanent MAC": "EC:F4:BB:C3:AF:AA",
50+
"Assigned MAC": "EC:F4:BB:C3:AF:AA",
51+
"Driver": "ixgbe",
52+
"Driver version": "5.9.4",
53+
"Firmware version": "0x8000095c, 19.5.12",
54+
"Bus Info": "0000:01:00.1",
55+
"PCI name": "0000:01:00.1",
56+
"PCI Slot": "embedded",
57+
"SMBIOS Device Type": "Ethernet",
58+
"SMBIOS Instance": "2",
59+
"SMBIOS Label": "Integrated NIC 2",
60+
"sysfs Label": "NIC2",
61+
"Embedded Index": "2",
62+
},
63+
"eth2": {
64+
"BIOS device": {"physical": "em3_1", "all_ethN": "eth2"},
65+
"Kernel name": "eth2",
66+
"Permanent MAC": "EC:F4:BB:C3:AF:AC",
67+
"Assigned MAC": "EC:F4:BB:C3:AF:AC",
68+
"Driver": "igb",
69+
"Driver version": "5.3.5.20",
70+
"Firmware version": "1.67, 0x80000fab, 19.5.12",
71+
"Bus Info": "0000:07:00.0",
72+
"PCI name": "0000:07:00.0",
73+
"PCI Slot": "embedded",
74+
"SMBIOS Device Type": "Ethernet",
75+
"SMBIOS Instance": "3",
76+
"SMBIOS Label": "Integrated NIC 3",
77+
"sysfs Label": "NIC3",
78+
"VPD Port": "3",
79+
"VPD Index": "1",
80+
"VPD PCI master": "0000:07:00.0",
81+
},
82+
"eth3": {
83+
"BIOS device": {"physical": "em4_1", "all_ethN": "eth3"},
84+
"Kernel name": "eth3",
85+
"Permanent MAC": "EC:F4:BB:C3:AF:AD",
86+
"Assigned MAC": "EC:F4:BB:C3:AF:AD",
87+
"Driver": "igb",
88+
"Driver version": "5.3.5.20",
89+
"Firmware version": "1.67, 0x80000fab, 19.5.12",
90+
"Bus Info": "0000:07:00.1",
91+
"PCI name": "0000:07:00.1",
92+
"PCI Slot": "embedded",
93+
"SMBIOS Device Type": "Ethernet",
94+
"SMBIOS Instance": "4",
95+
"SMBIOS Label": "Integrated NIC 4",
96+
"sysfs Label": "NIC4",
97+
"VPD Port": "4",
98+
"VPD Index": "1",
99+
"VPD PCI master": "0000:07:00.1",
100+
},
101+
}
102+
103+
def test_without_mock(self):
104+
"""Test all_devices_all_names using PopenWrapper() for Popen(), installed by setUp()"""
105+
# conftest.py redirects subprocess.Popen("/sbin/biosdevname") to call tests/data/biosdevname
106+
devices = all_devices_all_names()
107+
self.assertEqual(
108+
devices["eth0"]["BIOS device"], {"all_ethN": "eth0", "physical": "em1"}
109+
)
110+
self.assertEqual(devices, self.expected_devices)
111+
28112
def test(self):
113+
# sourcery skip: extract-method, inline-immediately-returned-variable, path-read
29114
with patch("xcp.net.biosdevname.Popen") as popen_mock:
115+
# pylint: disable=unspecified-encoding
30116
with open("tests/data/physical.biosdevname") as f:
31117
fake_output_1 = f.read()
32118
with open("tests/data/all_ethN.biosdevname") as f:
@@ -46,3 +132,4 @@ def test(self):
46132

47133
self.assertEqual(devices['eth0']['BIOS device'],
48134
{'all_ethN': 'eth0', 'physical': 'em1'})
135+
self.assertEqual(devices, self.expected_devices)

‎xcp/net/biosdevname.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ def __run_all_devices(policy = "physical"):
4242
"""
4343

4444
proc = Popen(["/sbin/biosdevname", "--policy", policy,
45-
"-d"], stdout=PIPE, stderr=PIPE)
45+
"-d"], stdout=PIPE, stderr=PIPE, universal_newlines=True)
4646

4747
stdout, stderr = proc.communicate()
4848

0 commit comments

Comments
 (0)