pci.PCIDevices(): Fix Popen("lspci -mn") for Py3 using universal_newlines=True

[bernhardkaindl]

This PR fixes one of the Python3 fixes requested in #21:

Two issues in the Python3 migration of xcp.pci weren't detected by the new test tests_videoclass() in tests/test_pci.py:

1. The use of mock to mock a call to Popen() (using open() didn't detect a usage bug.
2. The use of a the fake tests/data/pci.ids, not the real /usr/share/hwdata/pci.ids.

This PR fixes point 1.:

Add a new test case which does not use mock(), and fix the issue 1.

Mocking xcp.pci.subprocess.Popen("lspci -nm").stdout using open("tests/data/lspci-mn")
does not model the actual behavior of subprocess.Popen().stdout in Python3:

From: https://docs.python.org/3.6/library/subprocess.htmlpopen-constructor:

"If encoding or errors are specified, the file objects stdin, stdout and stderr
are opened in text mode with the specified encoding and errors, as described
above in Frequently Used Arguments.
If universal_newlines is True, they are opened in text mode with default
encoding. >>>Otherwise, they are opened as binary streams.<<<"

By it's nature, mocking Popen() does not validate it's correct use in xcp.pci
and therfore, it failed to detect the Python3 problem in xcp/pci.py

The validation of xcp.pci.PCIDevices() using self.PopenWrapper(), replacing
"lspci -mn" with "cat tests/data/lspci-mn" is able to detect such usage bugs.

Correctly modeling subprocess.Popen() using mock is nontrivial, and would require something like the testfixtures library and even that would still be a mock and use the actual subprocess class:
https://stackoverflow.com/questions/25692440/mocking-a-subprocess-call-in-python
https://chromium.googlesource.com/chromium/tools/depot_tools/+/refs/heads/main/tests/subprocess2_test.py

Mocking is better suited when the test knows how an API must be called, which isn't the case here, espcially the use of open("tests/data/lspci-mn") is wrong, which leads to the test not catching the Python3 bug:

with patch("xcp.pci.subprocess.Popen") as popen_mock, \
     open("tests/data/lspci-mn") as fake_data:   # <- The mock uses open() here,
                                                 # which is wrong and hard to fix using mock.
    popen_mock.return_value.stdout.__iter__ = Mock(return_value=iter(fake_data))
    devs = PCIDevices()

This PR therefore adds a new test case which does not use mock and changes the "mock test" to accept the fix.

But in principle after this PR is merged or as a final commit to it, the old mock-only test could be removed as obsolete and not up to the task.

[codecov]

Codecov Report

Merging #38 (2eb9203) into master (5c36c6e) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 2eb9203 differs from pull request most recent head 6bdd72b. Consider uploading reports for the commit 6bdd72b to get more accurate results

@@           Coverage Diff           @@
##           master      #38   +/-   ##
=======================================
  Coverage   69.27%   69.27%           
=======================================
  Files          20       20           
  Lines        3417     3417           
=======================================
  Hits         2367     2367           
  Misses       1050     1050           

Flag	Coverage Δ
unittest	69.27% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
xcp/pci.py	78.37% <100.00%> (ø)