CP-48198: close exited stdouts, fix warnings, use runpy, update cover…

…agerc (#98)

- Fix a number of unclosed file warnings in tests: close stdout of exited procs
- Use runpy.run_path() to execute xen-bugtool as a script:
  - tests/integration/test_*.py now also collect coverage of xen-bugtool
- Update .coveragerc: Run "coverage run && coverage report" to get coverage!
- Fix warnings from SonarLint, Codacy (e.g bandit), Sourcery
- Update configuration to be compatible with SonarCloud and upload to it
- Fix pyproject.toml to be compatible with "ruff check xen-bugtool"

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

.coveragerc

@@ -1,7 +1,20 @@
 [run]
-source =
-       xen-bugtool
-       tests/
+command_line = -m pytest --junitxml=.git/pytest.xml --cov-report=term-missing tests/unit
+# Default data file for "coverage run": Store coverage data in .git/.coverage
+data_file = .git/.coverage
+relative_files = True
+omit =
+    tests/integration/__init__.py
+    tests/mocks/xen/__init__.py
+    tests/mocks/xen/lowlevel/__init__.py
+    tests/mocks/xen/lowlevel/xc/__init__.py
+    tests/unit/__init__.py
+
+[coverage:html]
+directory = .git/coverage.html
+
+[coverage:xml]
+output = .git/coverage.xml
 
 [report]
 # Regular expressions for lines to exclude from consideration
@@ -30,5 +43,6 @@ exclude_lines =
 
 precision = 1
 include =
+    .
     xen-bugtool
     tests/*

.github/workflows/main.yml

@@ -178,6 +178,30 @@ jobs:
           # GitHub PR workflows are already always rebased to the target branch (on run):
           SKIP: no-commit-to-branch,check-branch-needs-rebase
 
+      - uses: frabert/replace-string-action@v2
+        id: get_sonarcloud_project_key
+        with:
+          pattern: '(\w+)/(\w+)'
+          replace-with: '$1_$2'
+          string: ${{ github.repository }}
+
+      - name: SonarCloud Scan
+        uses: SonarSource/sonarcloud-github-action@v2
+        if: ${{ env.SONAR_TOKEN }}
+        with:
+          args: >
+            -Dsonar.organization=${{ github.repository_owner }}
+            -Dsonar.projectKey=${{ steps.get_sonarcloud_project_key.outputs.replaced }}
+            -Dsonar.python.version=3.6
+            -Dsonar.python.coverage.reportPaths=.git/coverage.xml
+            -Dsonar.python.xunit.reportPath=.git/pytest.xml
+            -Dsonar.sources=.
+            -Dsonar.exclusions=tests/**
+            -Dsonar.tests=tests
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
+
       - name: Upload coverage reports to Codecov
         # If CODECOV_TOKEN is set, use the new codecov CLI to upload the coverage reports
         if: ${{ env.CODECOV_TOKEN && !cancelled() && github.event.pull_request.number }}

.pre-commit-config.yaml

@@ -152,13 +152,8 @@ repos:
     hooks:
     -   id: pytest
         name: check that the Xen-Bugtool Test Environment passes
-        entry: env PYTHONDEVMODE=yes python3 -m pytest tests/unit
-            --cov xen-bugtool
-            --cov tests/unit
-            --junitxml=.git/pytest.xml
-            --cov-report term-missing
-            --cov-report html:.git/coverage.html
-            --cov-report xml:.git/coverage.xml
+        entry: env PYTHONDEVMODE=yes sh -c
+            "coverage run && coverage xml && coverage html"
         require_serial: true
         pass_filenames: false
         language: python

.vscode/ltex.dictionary.en-US.txt

@@ -38,6 +38,7 @@ cli
 CLOEXEC
 clusterd
 Codecov
+codecovcli
 conf
 CONFD
 config
@@ -60,6 +61,7 @@ dirs
 docstrings
 dont
 dp
+Dsonar
 dunder
 efi
 efivars
@@ -82,6 +84,8 @@ filesystem
 filetype
 finalizer
 firstboot
+frabert
+frolvlad
 fs
 getgid
 getuid
@@ -116,6 +120,7 @@ ltex
 maxsplit
 mdadm
 mdstat
+Misha
 modinfo
 mountpoint
 mountpoints
@@ -129,6 +134,8 @@ networkd
 NEWNET
 NEWNS
 nonexisting
+noninteractive
+noqa
 NOSONAR
 NRPE
 nsswitch
@@ -162,6 +169,7 @@ pytest
 pytest's
 PYTHONDEVMODE
 PYTHONDONTWRITEBYTECODE
+PYTHONWARNINGS
 pytype
 rebase
 reportMissingImports
@@ -182,6 +190,7 @@ snmp
 snmpd
 softirqs
 softnet
+sonarcloud
 sourcery
 src
 startswith
@@ -213,6 +222,7 @@ tmpdata
 tmpdir
 tmpdir's
 tmpfs
+tokenless
 toolstack
 toplevel
 typeshed
@@ -259,6 +269,7 @@ xml
 xs
 xsconfig
 xsversion
+xunit
 xvda
 yestoall
 zipfile

README.md

@@ -1,5 +1,9 @@
 # Developer Documentation for Xen-Bugtool
 
+[![Quality Gate Status](https://sonarcloud.io/api/project_badges/measure?project=xenserver-next_status-report&metric=alert_status)](https://sonarcloud.io/summary/new_code?id=xenserver-next_status-report)
+[![Bugs](https://sonarcloud.io/api/project_badges/measure?project=xenserver-next_status-report&metric=bugs)](https://sonarcloud.io/summary/new_code?id=xenserver-next_status-report)
+[![Code Smells](https://sonarcloud.io/api/project_badges/measure?project=xenserver-next_status-report&metric=code_smells)](https://sonarcloud.io/summary/new_code?id=xenserver-next_status-report)
+
 This developer documentation provides detailed information about
 the development environment for `xen-bugtool`,
 a tool designed to assist with debugging XenServer issues.

pyproject.toml

@@ -3,7 +3,7 @@
 name = "xenserver-status-report"
 dynamic = ["version"]
 description = "Xenserver Status Report"
-requires-python = "2.7"
+requires-python = ">=2.7"
 license = "LGPL-2.1-only"
 keywords = ["xenserver", "xen-project"]
 authors = [

pytest.ini

@@ -6,6 +6,9 @@ filterwarnings=ignore:the imp module is deprecated
 # Enable live logging of the python log output, starting with log level INFO by default:
 log_cli=True
 log_cli_level=INFO
+required_plugins=
+    pyfakefs
+    pytest-mock
 # By default, run the tests in the tests directory:
 testpaths=tests/
 

sonar-project.properties

@@ -0,0 +1,15 @@
+sonar.organization=xenserver-next
+sonar.projectKey=xenserver-next_status-report
+sonar.python.version=3.6
+sonar.python.coverage.reportPaths=.git/coverage.xml
+sonar.python.xunit.reportPath=.git/pytest.xml
+# sonar.python.mypy.reportPaths=.git/mypy.xml
+# sonar.python.ruff.reportPaths=.git/ruff.xml
+# sonar.python.pylint.reportPaths=.git/pylint.xml
+
+# Patterns used to include some source files and only these ones in analysis.
+# By default, all files in project sources are included.
+# relative paths to source directories. More details and properties are described
+# in https://sonarcloud.io/documentation/project-administration/narrowing-the-focus/
+sonar.sources=.
+sonar.tests=tests

tests/integration/test_system_load.py

@@ -28,7 +28,7 @@ def test_system_load(output_archive_type="zip"):
     os.environ["PATH"] = "/var:" + os.environ["PATH"]
     with open("/var/sar", "w") as sar:
         sar.write("#!/bin/sh\nsleep 1;cat /etc/xensource-inventory\n")
-    os.chmod("/var/sar", 0o777)
+    os.chmod("/var/sar", 0o777)  # nosec
 
     run_bugtool_entry(output_archive_type, entry)
 

tests/integration/test_xenserver_config.py

@@ -40,17 +40,21 @@ def test_xenserver_config(output_archive_type):
     assert_content_from_dom0_template("etc/xensource-inventory")
 
     # Sample SNMP config files are currently not in the dom0_template!
-    # Reading them records the error message in the file content, do we want this?
-    # I think the "Failed to filter" is redundant in it.
-    # Maybe decide on a standardized error for missing files in bugtool?
-
-    # TODO: To be clarified or fixed as part of CP-46759 or a follow-up!
 
+    # sourcery skip: no-loop-in-tests
     for input_file in [
         "/etc/snmp/snmp.xs.conf",
         "/etc/snmp/snmpd.xs.conf",
         "/var/lib/net-snmp/snmpd.conf",
     ]:
+        #
+        # Here, we assert that the filter functions were called and tried to read
+        # the file (they fail here as the files are not part of the dom0_template).
+        #
+        # But this is fine, all we want to test here is that the filter was called:
+        # The filter functions are tested as part of the unit tests
+        # in tests/unit/test_snmp.py instead:
+        #
         assert_file(
             input_file.split("/")[-1].replace(".", "_") + ".out",
             # That's a very long error message an the 1st two parts are redundant:

tests/integration/utils.py

@@ -116,9 +116,9 @@ def extract(zip_or_tar_archive, archive_type):  # pragma: no cover
         if archive_type == "zip":
             archive = zipfile.ZipFile(zip_or_tar_archive)  # type: zipfile.ZipFile|tarfile.TarFile
         elif archive_type == "tar":
-            archive = tarfile.open(zip_or_tar_archive)
+            archive = tarfile.open(zip_or_tar_archive)  # NOSONAR
         elif archive_type == "tar.bz2":
-            archive = tarfile.open(zip_or_tar_archive, "r:bz2")
+            archive = tarfile.open(zip_or_tar_archive, "r:bz2")  # NOSONAR
         else:
             raise RuntimeError("Unsupported output archive type: %s" % archive_type)
         archive.extractall()

tests/unit/conftest.py

@@ -186,10 +186,10 @@ def isolated_bugtool(bugtool_log):
     """
 
     # Make the current cwd (a temporary directory) read-only:
-    os.chmod(".", 0o555)
+    os.chmod(".", 0o555)  # nosec
 
     yield bugtool_log  # runs the test function in the read-only directory
 
-    os.chmod(".", 0o777)  # restore write permissions (for cleanup)
+    os.chmod(".", 0o777)  # nosec # restore write permissions (for cleanup)
 
     # upon return, bugtool_log continues with its cleanup

tests/unit/test_main.py

@@ -196,11 +196,15 @@ def check_output(bugtool, captured, tmp_path, filename, filetype):
 
     # Extract the created output file into the tmp_path
     if filetype == "zip":
-        zipfile.ZipFile(filename).extractall(tmp_path)
+        zipfile.ZipFile(filename).extractall(tmp_path)  # nosec # noqa: B202 # NOSONAR
     elif filetype == "tar":
-        tarfile.TarFile.open(filename).extractall(tmp_path)
+        tar = tarfile.TarFile.open(filename)
+        tar.extractall(tmp_path)  # nosec
+        tar.close()
     elif filetype == "tar.bz2":
-        tarfile.TarFile.open(filename, "r:bz2").extractall(tmp_path)
+        tar = tarfile.TarFile.open(filename, "r:bz2")
+        tar.extractall(tmp_path)  # nosec
+        tar.close()
 
     output_directory = filename.replace(".%s" % filetype, "/")
 

tests/unit/test_output.py

@@ -27,6 +27,9 @@ def mock_data_collector(capability):
     raise Exception("mock data collector failed")
 
 
+ETC_PASSWD = "/etc/passwd"
+
+
 def assert_valid_inventory_schema(inventory_tree):
     """Assert that the passed inventory validates against the inventory schema"""
 
@@ -55,21 +58,21 @@ def assert_mock_bugtool_plugin_output(temporary_directory, subdir, names):
     # Will be refactored to be more easy in a separate commit soon:
     assert_valid_inventory_schema(parse(extracted + "inventory.xml"))
     with open(extracted + "proc_version.out") as proc_version:
-        assert proc_version.read()[:14] == "Linux version "
+        assert proc_version.read().startswith("Linux version ")
     with open(extracted + "ls-l-%etc.out") as etc:
-        assert etc.read()[:6] == "total "
+        assert etc.read().startswith("total ")
     with open(extracted + "proc/self/status") as status:
-        assert status.read()[:5] == "Name:"
+        assert status.read().startswith("Name:")
     with open(extracted + "proc/sys/fs/epoll/max_user_watches") as max_user_watches:
         assert int(max_user_watches.read()) > 0
     with open(extracted + "etc/group") as group:
         assert group.readline() == "root:x:0:\n"
 
     # Check the contents of the sub-archive "etc/passwd.tar":
-    with tarfile.TarFile(extracted + "etc/passwd.tar") as tar:
-        assert tar.getnames() == [subdir + "/etc/passwd"]
+    with tarfile.TarFile(extracted + ETC_PASSWD + ".tar") as tar:
+        assert tar.getnames() == [subdir + ETC_PASSWD]
         # TarFile.extractfile() does not support context managers on Python2:
-        passwd = tar.extractfile(subdir + "/etc/passwd")
+        passwd = tar.extractfile(subdir + ETC_PASSWD)
         assert passwd
         assert passwd.readline() == b"root:x:0:0:root:/root:/bin/bash\n"
         passwd.close()
@@ -82,7 +85,7 @@ def minimal_bugtool(bugtool, dom0_template, archive, subdir, mocker):
     # Load the mock plugin from dom0_template and process the plugin's caps:
     bugtool.PLUGIN_DIR = dom0_template + "/etc/xensource/bugtool"
     bugtool.entries = ["mock"]
-    archive.declare_subarchive("/etc/passwd", subdir + "/etc/passwd.tar")
+    archive.declare_subarchive(ETC_PASSWD, subdir + ETC_PASSWD + ".tar")
     # For code coverage: This sub-archive will not be created as it has no file
     archive.declare_subarchive("/not/existing", subdir + "/not_created.tar")
     bugtool.load_plugins(just_capabilities=False)

xen-bugtool

@@ -515,9 +515,7 @@ class StringIOmtime(io.BytesIO):
 
 
 def no_unicode(x):
-    if isinstance(x, unicode_type):
-        return x.encode('utf-8')
-    return x
+    return x.encode("utf-8") if isinstance(x, unicode_type) else x
 
 
 @contextmanager
@@ -2184,7 +2182,7 @@ def disk_list():
         with open(PROC_PARTITIONS, "r") as f:
             f.readline()
             f.readline()
-            for line in f.readlines():
+            for line in f:
                 major, _, _, name = line.split()
                 if int(major) < 254 and not partition_re.match(name):
                     disks.append(name)
@@ -2251,9 +2249,12 @@ class ProcOutput:
 
     def read_line(self):
         assert self.running
+        assert self.proc
+        assert self.proc.stdout
         line = self.proc.stdout.readline()
         if not line:
             # process exited
+            self.proc.stdout.close()
             self.status = self.proc.wait()
             self.proc = None
             self.running = False