Merge pull request #131 from xenserver-next/private/rossla/CA-392459

CA-392459: Avoid opening /dev/mem when calling biosdevname

Author: bernhardkaindl

.github/workflows/main.yml

@@ -36,11 +36,11 @@ jobs:
           os: ubuntu-22.04
     runs-on: ${{ matrix.os }}
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
         with:
           fetch-depth: 0  # Needed by diff-cover to get the changed lines: origin/master..HEAD
       - name: Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python-version }}
 
@@ -73,13 +73,59 @@ jobs:
           python3.8 -m pip install 'virtualenv<20.22' 'tox==4.5.1' tox-gh-actions
           tox --workdir .github/workflows/.tox --recreate
 
-      - name: Upload coverage reports to Codecov
-        if: ${{ matrix.os == 'ubuntu-20.04' && github.actor != 'nektos/act'}}
+
+      # The new reliable Codecov upload requires Codecov to query the GitHub API to check
+      # the repo and the commit. The repo (or organisation) owner needs to login to
+      # codev, generated the CODECOV_TOKEN and save it as a secret in the ORG or the repo:
+      # https://docs.codecov.com/docs/adding-the-codecov-token
+
+      # Links to get and set the token:
+      # Get the CODECOV_TOKEN: https://app.codecov.io/gh/xenserver/python-libs/settings
+      # Set the CODE_COV_TOKEN: https://github.com/xenserver/python-libs/settings/secrets/actions
+
+      # Without it, the API calls are rate-limited by GitHub, and the upload may fail:
+      # https://github.com/codecov/feedback/issues/126#issuecomment-1932658904
+      #
+      - name: Upload coverage reports to Codecov (fallback, legacy Node.js 16 action)
+        # If CODECOV_TOKEN is not set, use the legacy tokenless Codecov action:
+        env:
+          CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
+        if: |
+          !env.CODECOV_TOKEN && !cancelled() &&
+          matrix.os == 'ubuntu-20.04' && github.actor != 'nektos/act' &&
+          ( github.event.pull_request.number || github.ref == 'refs/heads/master' )
         uses: codecov/codecov-action@v3
         with:
           directory: .github/workflows/.tox/py38-covcombine-check/log
           env_vars: OS,PYTHON
-          fail_ci_if_error: true
+          # Use fail_ci_if_error: false as explained the big comment above:
+          # Not failing this job in this case is ok because the tox CI checks also contain
+          # a diff-cover check which would fail on changed lines missing coverage.
+          fail_ci_if_error: false
           flags: unittest
           name: py27-py38-combined
           verbose: true
+
+      - name: Upload coverage reports to Codecov (used when secrets.CODECOV_TOKEN is set)
+        # If CODECOV_TOKEN is set, use the new Codecov CLI to upload the coverage reports
+        env:
+          CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
+        if: |
+          env.CODECOV_TOKEN && !cancelled() && github.actor != 'nektos/act' &&
+          ( github.event.pull_request.number || github.ref == 'refs/heads/master' )
+        run: >
+          set -euxv;
+          mv .github/workflows/.tox/py38-covcombine-check/log/coverage.xml cov.xml;
+          curl -O https://cli.codecov.io/latest/linux/codecov; sudo chmod +x codecov;
+          ./codecov upload-process --report-type coverage
+          --name "CLI Upload for ${{ env.PYTHON_VERSION }}"
+          --git-service github --fail-on-error --file cov.xml --disable-search
+          --flag python${{ env.PYTHON_VERSION }}
+        continue-on-error: false  # Fail the job if the upload with CODECOV_TOKEN fails
+
+
+      - name: Upload coverage reports to Coveralls
+        env:
+          COVERALLS_FLAG_NAME: ${{ format('python{0}', steps.python.outputs.python-version ) }}
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: pip install coveralls && coveralls --service=github && coveralls --finish

.pre-commit-config.yaml

@@ -53,20 +53,28 @@ repos:
         args: [--branch, master]
         always_run: true
 -   repo: https://github.com/akaihola/darker
-    rev: 1.7.1
+    rev: 1.7.3
     hooks:
     -   id: darker
-        args: [--isort, -tpy33]
+        args: [--isort, -S, -tpy36]
         verbose: true
         additional_dependencies:
-          - black
           - isort
+
+
+-   repo: https://github.com/pre-commit/mirrors-mypy
+    rev: v1.8.0
+    hooks:
+    -   id: mypy
+        additional_dependencies:
+        - pytest-subprocess
+        - types-mock
+        - types-six
+
+
 -   repo: https://github.com/rcmdnk/pyproject-pre-commit
     rev: v0.0.12
     hooks:
-    -   id: mypy
-        args: [--ignore-missing-imports] # --config-file, "mypy.ini"(config is in pyproject.toml)
-        pass_filenames: true
     -   id: shellcheck
     -   id: mdformat-check
         exclude: README-Unicode.md

pytest.ini

@@ -9,6 +9,7 @@
 # not support ;python_version<=3.0 or ;python_version>3.0. Therefore, it can
 # only list plugins available for all tested python versions (2.7, 3.6 ... 3.11):
 required_plugins =
+    pytest_httpserver
     pytest-forked
     pytest-localftpserver
     pytest-pythonpath

tests/test_biosdevname.py

@@ -45,8 +45,8 @@ def test(self):
         # check after the fact that we mocked the proper calls
         self.assertEqual(popen_mock.call_count, 2)
         calls = popen_mock.call_args_list
-        self.assertEqual(calls[0].args[0], ['/sbin/biosdevname', '--policy', 'physical', '-d'])
-        self.assertEqual(calls[1].args[0], ['/sbin/biosdevname', '--policy', 'all_ethN', '-d'])
+        self.assertEqual(calls[0].args[0], ['/sbin/biosdevname', '--policy', 'physical', '-d', '-x'])
+        self.assertEqual(calls[1].args[0], ['/sbin/biosdevname', '--policy', 'all_ethN', '-d', '-x'])
         popen_kwargs = {"stdout": PIPE, "stderr": PIPE, "universal_newlines": True}
         self.assertEqual(calls[0].kwargs, popen_kwargs)
         self.assertEqual(calls[1].kwargs, popen_kwargs)

tests/test_httpaccessor.py

@@ -1,5 +1,6 @@
-"""Test xcp.accessor.HTTPAccessor using a local pure-Python http(s)server fixture"""
 # -*- coding: utf-8 -*-
+"""Test xcp.accessor.HTTPAccessor using a local pure-Python http(s)server fixture"""
+
 import base64
 import sys
 from contextlib import contextmanager
@@ -8,7 +9,7 @@
 
 from six.moves import urllib  # pyright: ignore
 
-from xcp.accessor import createAccessor, HTTPAccessor
+from xcp.accessor import HTTPAccessor, createAccessor
 
 from .httpserver_testcase import ErrorHandler, HTTPServerTestCase, Response
 
@@ -42,6 +43,7 @@ def http_get_request_data(self, url, read_file, error_handler):
 
     def assert_http_get_request_data(self, url, read_file, error_handler):
         # type:(str, str, ErrorHandler) -> HTTPAccessor
+        # pyre-ignore[23]: silence false positive
         with self.http_get_request_data(url, read_file, error_handler) as (httpaccessor, ref):
             http_accessor_filehandle = httpaccessor.openAddress(read_file)
             if sys.version_info >= (3, 0):

tests/test_ifrename_dynamic.py

@@ -20,8 +20,8 @@ def setUp(self):
         openLog(self.logbuf, logging.NOTSET)
 
     def tearDown(self):
-        self.logbuf.close()
         closeLogs()
+        self.logbuf.close()
 
     def test_null(self):
 
@@ -91,7 +91,6 @@ def setUp(self):
         openLog(self.logbuf, logging.NOTSET)
 
     def tearDown(self):
-
         closeLogs()
         self.logbuf.close()
 
@@ -167,7 +166,6 @@ def setUp(self):
         openLog(self.logbuf, logging.NOTSET)
 
     def tearDown(self):
-
         closeLogs()
         self.logbuf.close()
 

tests/test_mountingaccessor.py

@@ -137,7 +137,7 @@ def open_text(accessor, location, fs, text):
     assert isinstance(accessor, xcp.accessor.MountingAccessorTypes)
     name = "textfile"
     path = location + "/" + name
-    assert fs.create_file(path, contents=text)
+    assert fs.create_file(path, contents=text, encoding="utf-8")
     assert accessor.access(name)
     with accessor.openText(name) as textfile:
         assert not isinstance(textfile, bool)

xcp/net/biosdevname.py

@@ -40,7 +40,7 @@ def __run_all_devices(policy = "physical"):
     """
 
     proc = Popen(["/sbin/biosdevname", "--policy", policy,
-                  "-d"], stdout=PIPE, stderr=PIPE, universal_newlines=True)
+                  "-d", "-x"], stdout=PIPE, stderr=PIPE, universal_newlines=True)
 
     stdout, stderr = proc.communicate()