CI: Improve unit tests to test the bugtool's main(): Coverage: 81%!

Add tests/unit/test_main.py which also uses test functions which were added for earlier tests to test the main function of bugtool to produce the bugtool output archives for all types: zip, tar, and tar.bz2. The main test checks the output of the bugtool application to ensure that it matches the expected output. It compares the captured output with the expected output and performs various assertions to validate the output. It extracts the output files from the archive and checks that the xAPI db and the inventory.xml. Tested output: - The start and end of the collected bugtool messages from captured.out - The inventory.xml file is checked to be validated using its XML schema. - The xapi-db.xml is checked to have secrets filtered using a dummy xapi db Signed-off-by: Bernhard Kaindl <[email protected]>
xenserver · Jan 12, 2024 · 0783b2f · 0783b2f
1 parent 1aae840
commit 0783b2f
Show file tree

Hide file tree

Showing 10 changed files with 447 additions and 74 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -14,6 +14,13 @@ name: "GitHub CI"
 # https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/about-status-checks#skipping-and-requesting-checks-for-individual-commits
 on: [push, pull_request]
 
+# Docs: https://github.com/marketplace/actions/commit-status-updater#workflow-permissions
+# and more info on https://github.com/actions/first-interaction/issues/10
+# Allow actions in PRs from other repos to create a comment and update status:
+permissions:
+  pull-requests: write  # creating a comment
+  statuses: write  #  # updating commit status
+
 # Cancel a currently running workflow from the same PR, branch or tag
 # when a new workflow is triggered:
 # https://stackoverflow.com/questions/66335225/how-to-cancel-previous-runs-in-the-pr-when-you-push-new-commitsupdate-the-curre
@@ -63,7 +70,39 @@ jobs:
       - name: Run pylint-1.9.4 for pylint --py3k linting (configured in .pylintrc)
         run: python2 -m pylint xen-bugtool
       - name: Run python2 -m pytest to execute all unit and integration tests
-        run: python2 -m pytest -v -rA
+        run: >
+          python2 -m pytest -v -rA
+          --cov xen-bugtool
+          --cov tests/
+          --junitxml=.git/pytest27.xml
+          --cov-report term-missing
+          --cov-report html:.git/coverage27.html
+          --cov-report xml:.git/coverage27.xml
+
+      - name: Upload coverage reports to Codecov for the Python 2.7 tests
+        if: ${{ github.actor != 'nektos/act' }}
+        uses: codecov/codecov-action@v3
+        env:
+          PYTHON: python2.7
+        with:
+          directory: .git
+          files: coverage27.xml
+          flags: python2.7
+          env_vars: OS,PYTHON
+          fail_ci_if_error: true
+          name: coverage27
+          verbose: true
+
+      - name: Add Pytest 2.7 coverage comment (if write permission is available)
+        if: ${{ ! github.event.pull_request.head.repo.fork }}
+        uses: MishaKav/pytest-coverage-comment@main
+        continue-on-error: true
+        with:
+          junitxml-path: .git/pytest27.xml
+          pytest-xml-coverage-path: .git/coverage27.xml
+          unique-id-for-comment: pytest-coverage-python27
+          title: Pytest Code coverage comment for Python 2.7
+
 
   pre-commit:
     name: "Python3: Pre-Commit Suite"
@@ -76,6 +115,7 @@ jobs:
 
       # https://www.python4data.science/en/latest/productive/git/advanced/hooks/ci.html
       - uses: actions/setup-python@v4
+        id: python
         with:
           python-version: '3.10'
           cache: 'pip'
@@ -95,22 +135,27 @@ jobs:
           # Skip the no-commit-to-branch check inside of GitHub CI (for CI on merge to master)
           SKIP: no-commit-to-branch
 
-      - name: Pytest coverage comment
-        if: ${{ github.actor != 'nektos/act' }}
-        uses: MishaKav/pytest-coverage-comment@main
-        with:
-          junitxml-path: .git/pytest.xml
-          pytest-xml-coverage-path: .git/coverage.xml
-          unique-id-for-comment: pre-commit-coverage
-          title: https://github.com/marketplace/actions/pytest-coverage-comment
-
       - name: Upload coverage reports to Codecov
         if: ${{ github.actor != 'nektos/act' }}
         uses: codecov/codecov-action@v3
         with:
           directory: .git
           env_vars: OS,PYTHON
           fail_ci_if_error: true
-          flags: unittest
+          flags: >
+            unittest,
+            ${{ format('python{0}', steps.python.outputs.python-version ) }}
           name: coverage
           verbose: true
+
+      - name: Add Pytest coverage comment (if write permission is available)
+        if: ${{ ! github.event.pull_request.head.repo.fork }}
+        uses: MishaKav/pytest-coverage-comment@main
+        continue-on-error: true
+        with:
+          junitxml-path: .git/pytest.xml
+          pytest-xml-coverage-path: .git/coverage.xml
+          unique-id-for-comment: pre-commit-coverage
+          title: >
+            Python3 coverage comment from
+            https://github.com/marketplace/actions/pytest-coverage-comment
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -13,6 +13,7 @@
 #
 # When this works, you can enable it as the pre-commit hook for this git clone:
 # $ pre-commit install
+# $ pre-commit install --hook-type pre-push
 #
 # You can skip checks if you commit very often you don't want them to run, e.g:
 # export SKIP=mypy,pylint;git commit -m "quick save" (or for --amend)
@@ -44,7 +45,7 @@ repos:
         name: 'check and fix files to have no trailing whitespace'
     -   id: end-of-file-fixer
         name: 'check and fix that files have a trailing newline'
-        exclude: tests/integration/sar-file-collection.test.sh
+        exclude: tests/integration/dom0-template
     -   id: mixed-line-ending
         args: ['--fix=lf']
         name: 'check and fix that line endings are line feeds'
@@ -58,45 +59,10 @@ repos:
     -   id: check-merge-conflict
     -   id: check-yaml
         name: 'check the syntax of yaml files'
-# Run "python3 -m pip install -r requirements-dev.txt" to run pytest or use "git commit --no-verify":
--   repo: local
-    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
-        require_serial: true
-        pass_filenames: false
-        language: python
-        types: [python]
-        additional_dependencies: [coverage, defusedxml, pytest-coverage, lxml, XenAPI]
-
-
-    -   id: diff-cover
-        name: check that that the changed lines are covered by tests
-        entry: diff-cover --ignore-whitespace --compare-branch=origin/master
-            --show-uncovered --html-report .git/coverage-diff.html
-            --fail-under 100 .git/coverage.xml
-        require_serial: true
-        pass_filenames: false
-        language: python
-        types: [python]
-        additional_dependencies: [diff-cover]
 
+# The fast checks are first, to fix them first,
+# pytest with code coverage at the end.
 
-    -   id: coverage-report
-        name: check coverage report for minimum overall coverage
-        entry: coverage report --fail-under 55 #| tee .git/coverage.txt
-        require_serial: true
-        pass_filenames: false
-        language: python
-        types: [python]
-        additional_dependencies: [coverage]
 -   repo: https://github.com/PyCQA/autoflake
     rev: v2.2.1
     hooks:
@@ -105,13 +71,17 @@ repos:
         args: ["--in-place", "--remove-unused-variables", "--remove-all-unused-imports"]
         language: python
         files: \.py$
+
+
 -   repo: https://github.com/psf/black
     rev: 23.11.0
     hooks:
     -   id: black
         name: Ensure that all files (excluding xen-bugtool itself) are black-formatted
         args: [--safe, --quiet]
         exclude: xen-bugtool
+
+
 -   repo: https://github.com/akaihola/darker
     rev: 1.7.2
     hooks:
@@ -123,20 +93,73 @@ repos:
         args: [darker-xen-bugtool]
         additional_dependencies:
           - isort
+
+
 -   repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v1.7.1
+    rev: v1.8.0
     hooks:
     -   id: mypy
         name: Run mypy to check e.g. that all expected arguments are passed to functions etc
         additional_dependencies: [defusedxml, pytest, types-lxml]
+
+
 -   repo: https://github.com/pycqa/pylint
-    rev: v3.0.2
+    rev: v3.0.3
     hooks:
     -   id: pylint
         name: Run pylint to check that docstrings are added (and all other enabled checks)
         log_file: ".git/pre-commit-pylint.log"
+
+
+# Run "python3 -m pip install -r requirements-dev.txt" to run pytest or use "git commit --no-verify":
+-   repo: local
+    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
+        require_serial: true
+        pass_filenames: false
+        language: python
+        types: [python]
+        additional_dependencies:
+        - coverage
+        - defusedxml
+        - pytest-coverage
+        - pytest-mock
+        - lxml
+        - XenAPI
+
+
+    -   id: diff-cover
+        name: check that that the changed lines are covered by tests
+        entry: diff-cover --ignore-whitespace --compare-branch=origin/master
+            --show-uncovered --html-report .git/coverage-diff.html
+            --fail-under 100 .git/coverage.xml
+        require_serial: true
+        pass_filenames: false
+        language: python
+        types: [python]
+        additional_dependencies: [diff-cover]
+
+
+    -   id: coverage-report
+        name: check coverage report for minimum overall coverage
+        entry: coverage report --fail-under 55 #| tee .git/coverage.txt
+        require_serial: true
+        pass_filenames: false
+        language: python
+        types: [python]
+        additional_dependencies: [coverage]
+
+
 -   repo: https://github.com/RobertCraigie/pyright-python
-    rev: v1.1.339
+    rev: v1.1.345
     hooks:
     -   id: pyright
         name: Run pyright to check the unit tests for any typing warnings (use for bugtool later)

diff --git a/.pylintrc b/.pylintrc
@@ -64,7 +64,8 @@ py-version=2.7
 [BASIC]
 
 # Good variable names which should always be accepted, separated by a comma.
-good-names=i,
+good-names=b,
+           i,
            fd,
            j,
            k,
@@ -161,6 +162,7 @@ disable=anomalous-backslash-in-string,
         invalid-name,
         invalid-name,
         len-as-condition,
+        missing-type-doc,  # better do typing than type in the param docstring
         no-else-break,
         no-else-return,
         no-name-in-module,

diff --git a/requirements-dev.txt b/requirements-dev.txt
@@ -1,2 +1,3 @@
 lxml
-pytest
+pytest-coverage
+pytest-mock
diff --git a/tests/integration/sar-file-collection.test.sh b/tests/integration/sar-file-collection.test.sh
@@ -74,4 +74,4 @@ popd
 # Check that the tar and zip outputs are identical (except for date and uptime):
 sed -i "s/\\<$tar_basename\\>/$zip_basename/" tar/inventory.xml
 sed -i 's/date="[^"]*"//;s/uptime="[^"]*"//' {tar,zip}/inventory.xml
-diff -rNu tar zip
+diff -rNu tar zip
diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py
@@ -1,5 +1,6 @@
 """tests/unit/conftest.py: pytest fixtures for unit-testing functions in the xen-bugtool script"""
 import os
+import sys
 
 import pytest
 
@@ -14,18 +15,15 @@ def testdir():
 def dom0_template(testdir):
     """Test fixture to get the directory of the dom0 template and adding it's /usr/sbin to the PATH"""
     dom0_root_dir = testdir + "/../integration/dom0-template"
-    os.environ["PATH"] = dom0_root_dir + "/usr/sbin"  # for modinfo, mdadm, etc
     return dom0_root_dir
 
 
 @pytest.fixture(scope="session")
-def imported_bugtool(testdir):
+def imported_bugtool(testdir, dom0_template):
     """Test fixture to import the xen-bugtool script as a module for executing unit tests on functions"""
 
     # This uses the deprecated imp module because it needs also to run with Python2.7 for now:
     def import_from_file(module_name, file_path):
-        import sys
-
         if sys.version_info.major == 2:  # pragma: no cover
             import imp  # pylint: disable=deprecated-module  # pyright: ignore[reportMissingImports]
 
@@ -47,6 +45,9 @@ def import_from_file(module_name, file_path):
 
     bugtool = import_from_file("xen-bugtool", testdir + "/../../xen-bugtool")
     bugtool.ProcOutput.debug = True
+    # Prepend tests/mocks to force the use of our mock xen.lowlevel.xc module:
+    sys.path.insert(0, testdir + "/../mocks")
+    os.environ["PATH"] = dom0_template + "/usr/sbin"  # for modinfo, mdadm, etc
     return bugtool
 
 
@@ -55,4 +56,5 @@ def bugtool(imported_bugtool):
     """Test fixture for unit tests, initializes the bugtool data dict for each test"""
     # Init import_bugtool.data, so each unit test function gets it pristine:
     imported_bugtool.data = {}
+    sys.argv = ["xen-bugtool", "--unlimited"]
     return imported_bugtool