diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 7e0bc4a8..7047b4af 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -20,6 +20,19 @@ env: PIP_DISABLE_PIP_VERSION_CHECK: "1" # Reduce noise in logs jobs: + pre-commit: + env: + SKIP: pytest,no-commit-to-branch + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-python@v4 + with: + python-version: 3.11 + - uses: pre-commit/action@v3.0.1 + - uses: pre-commit-ci/lite-action@v1.0.3 + if: always() + test: strategy: # See: https://github.com/xenserver/python-libs/pull/26#discussion_r1179482169 @@ -28,12 +41,12 @@ jobs: fail-fast: false matrix: include: - - python-version: '3.6' - os: ubuntu-20.04 - - python-version: '3.10' - os: ubuntu-22.04 - python-version: '3.11' os: ubuntu-22.04 + - python-version: '3.12' + os: ubuntu-22.04 + - python-version: '3.13' + os: ubuntu-22.04 runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 @@ -58,21 +71,6 @@ jobs: pip install 'virtualenv<20.22' 'tox==4.5.1' tox-gh-actions tox --workdir .github/workflows/.tox --recreate - # tox >= 4.0.0 is needed for using optional-dependencies from pyproject.toml, which is - # is not available for python <= 3.6, so use the python3.8 of Ubuntu-20.04 to install it: - - name: Run tox for 3.6 and 3.8 on ${{ matrix.os }}'s 3.8 to get 'extras' from pyproject.toml) - if: ${{ matrix.python-version == 2.7 || matrix.python-version == 3.6 }} - run: | - set -xv;curl -sSL https://bootstrap.pypa.io/get-pip.py -o get-pip.py - python3.8 get-pip.py - # The alternative is installing python3-pip but we don't need full pip function for now: - # sudo apt-get update && sudo apt-get install -y python3-pip - # Let tox-gh-actions get the environment(s) to run tests with from tox.ini: - # Use tox==4.5.1: tox>=4 is needed for reading the extras from pyproject.toml - # Warning: tox>=4.5.2 depends on virutalenv>=20.23, which breaks Python 2.7: - python3.8 -m pip install 'virtualenv<20.22' 'tox==4.5.1' tox-gh-actions - tox --workdir .github/workflows/.tox --recreate - - name: Select the coverage file for upload if: | ( matrix.python-version == '3.6' || matrix.python-version == '3.11' ) && diff --git a/.markdownlint.yaml b/.markdownlint.yaml new file mode 100644 index 00000000..de8a471d --- /dev/null +++ b/.markdownlint.yaml @@ -0,0 +1,259 @@ +# Example markdownlint configuration with all properties set to their default value + +# Default state for all rules +default: true + +# Path to configuration file to extend +extends: null + +# MD001/heading-increment : Heading levels should only increment by one level at a time : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md001.md +MD001: true + +# MD003/heading-style : Heading style : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md003.md +MD003: + # Heading style + style: "consistent" + +# MD004/ul-style : Unordered list style : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md004.md +MD004: + # List style + style: "consistent" + +# MD005/list-indent : Inconsistent indentation for list items at the same level : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md005.md +MD005: true + +# MD007/ul-indent : Unordered list indentation : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md007.md +MD007: + # Spaces for indent + indent: 2 + # Whether to indent the first level of the list + start_indented: false + # Spaces for first level indent (when start_indented is set) + start_indent: 2 + +# MD009/no-trailing-spaces : Trailing spaces : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md009.md +MD009: + # Spaces for line break + br_spaces: 2 + # Allow spaces for empty lines in list items + list_item_empty_lines: false + # Include unnecessary breaks + strict: false + +# MD010/no-hard-tabs : Hard tabs : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md010.md +MD010: + # Include code blocks + code_blocks: true + # Fenced code languages to ignore + ignore_code_languages: [] + # Number of spaces for each hard tab + spaces_per_tab: 1 + +# MD011/no-reversed-links : Reversed link syntax : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md011.md +MD011: true + +# MD012/no-multiple-blanks : Multiple consecutive blank lines : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md012.md +MD012: + # Consecutive blank lines + maximum: 1 + +# MD013/line-length : Line length : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md013.md +MD013: + # Number of characters + line_length: 362 + # Number of characters for headings + heading_line_length: 210 + # Number of characters for code blocks + code_block_line_length: 278 + # Include code blocks + code_blocks: true + # Include tables + tables: true + # Include headings + headings: true + # Strict length checking + strict: false + # Stern length checking + stern: false + +# MD014/commands-show-output : Dollar signs used before commands without showing output : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md014.md +MD014: true + +# MD018/no-missing-space-atx : No space after hash on atx style heading : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md018.md +MD018: true + +# MD019/no-multiple-space-atx : Multiple spaces after hash on atx style heading : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md019.md +MD019: true + +# MD020/no-missing-space-closed-atx : No space inside hashes on closed atx style heading : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md020.md +MD020: true + +# MD021/no-multiple-space-closed-atx : Multiple spaces inside hashes on closed atx style heading : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md021.md +MD021: true + +# MD022/blanks-around-headings : Headings should be surrounded by blank lines : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md022.md +MD022: + # Blank lines above heading + lines_above: 1 + # Blank lines below heading + lines_below: 1 + +# MD023/heading-start-left : Headings must start at the beginning of the line : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md023.md +MD023: true + +# MD024/no-duplicate-heading : Multiple headings with the same content : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md024.md +MD024: + # Only check sibling headings + allow_different_nesting: false + # Only check sibling headings + siblings_only: false + +# MD025/single-title/single-h1 : Multiple top-level headings in the same document : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md025.md +MD025: + # Heading level + level: 1 + # RegExp for matching title in front matter + front_matter_title: "^\\s*title\\s*[:=]" + +# MD026/no-trailing-punctuation : Trailing punctuation in heading : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md026.md +MD026: + # Punctuation characters + punctuation: ".,;:!。,;:!" + +# MD027/no-multiple-space-blockquote : Multiple spaces after blockquote symbol : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md027.md +MD027: true + +# MD028/no-blanks-blockquote : Blank line inside blockquote : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md028.md +MD028: true + +# MD029/ol-prefix : Ordered list item prefix : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md029.md +MD029: + # List style + style: "one_or_ordered" + +# MD030/list-marker-space : Spaces after list markers : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md030.md +MD030: + # Spaces for single-line unordered list items + ul_single: 1 + # Spaces for single-line ordered list items + ol_single: 1 + # Spaces for multi-line unordered list items + ul_multi: 1 + # Spaces for multi-line ordered list items + ol_multi: 1 + +# MD031/blanks-around-fences : Fenced code blocks should be surrounded by blank lines : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md031.md +MD031: + # Include list items + list_items: true + +# MD032/blanks-around-lists : Lists should be surrounded by blank lines : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md032.md +MD032: true + +# MD033/no-inline-html : Inline HTML : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md033.md +MD033: + # Allowed elements + allowed_elements: [img, kbd] + +# MD034/no-bare-urls : Bare URL used : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md034.md +MD034: true + +# MD035/hr-style : Horizontal rule style : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md035.md +MD035: + # Horizontal rule style + style: "consistent" + +# MD036/no-emphasis-as-heading : Emphasis used instead of a heading : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md036.md +MD036: + # Punctuation characters + punctuation: ".,;:!?。,;:!?" + +# MD037/no-space-in-emphasis : Spaces inside emphasis markers : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md037.md +MD037: true + +# MD038/no-space-in-code : Spaces inside code span elements : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md038.md +MD038: true + +# MD039/no-space-in-links : Spaces inside link text : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md039.md +MD039: true + +# MD040/fenced-code-language : Fenced code blocks should have a language specified : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md040.md +MD040: + # List of languages + allowed_languages: [] + # Require language only + language_only: false + +# MD041/first-line-heading/first-line-h1 : First line in a file should be a top-level heading : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md041.md +MD041: + # Heading level + level: 1 + # RegExp for matching title in front matter + front_matter_title: "^\\s*title\\s*[:=]" + +# MD042/no-empty-links : No empty links : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md042.md +MD042: true + +# MD044/proper-names : Proper names should have the correct capitalization : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md044.md +MD044: + # List of proper names + names: [] + # Include code blocks + code_blocks: true + # Include HTML elements + html_elements: true + +# MD045/no-alt-text : Images should have alternate text (alt text) : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md045.md +MD045: false + +# MD046/code-block-style : Code block style : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md046.md +MD046: + # Block style + style: "consistent" + +# MD047/single-trailing-newline : Files should end with a single newline character : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md047.md +MD047: true + +# MD048/code-fence-style : Code fence style : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md048.md +MD048: + # Code fence style + style: "consistent" + +# MD049/emphasis-style : Emphasis style : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md049.md +MD049: + # Emphasis style + style: "consistent" + +# MD050/strong-style : Strong style : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md050.md +MD050: + # Strong style + style: "consistent" + +# MD051/link-fragments : Link fragments should be valid : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md051.md +MD051: true + +# MD052/reference-links-images : Reference links and images should use a label that is defined : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md052.md +MD052: + # Include shortcut syntax + shortcut_syntax: false + +# MD053/link-image-reference-definitions : Link and image reference definitions should be needed : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md053.md +MD053: + # Ignored definitions + ignored_definitions: + - "//" + +# MD054/link-image-style : Link and image style : https://github.com/DavidAnson/markdownlint/blob/v0.32.1/doc/md054.md +MD054: + # Allow autolinks + autolink: true + # Allow inline links and images + inline: true + # Allow full reference links and images + full: true + # Allow collapsed reference links and images + collapsed: true + # Allow shortcut reference links and images + shortcut: true + # Allow URLs as inline links + url_inline: true diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4dea24a9..9a7d227b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -37,7 +37,7 @@ # https://github.com/dexpota/cheatsheets/blob/master/pre-commit exclude: "^tests/data" fail_fast: true -default_stages: [commit] +default_stages: [pre-commit] repos: - repo: local hooks: @@ -47,7 +47,7 @@ repos: types: [binary] language: fail - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.6.0 + rev: v5.0.0 hooks: - id: no-commit-to-branch args: [--branch, master] @@ -62,8 +62,16 @@ repos: - isort +- repo: https://github.com/pre-commit/pygrep-hooks + rev: v1.10.0 # Use the ref you want to point at + hooks: + # Enforce that `# type: ignore` annotations always occur with specific codes. + # Sample annotation: # type: ignore[attr-defined,name-defined] + - id: python-check-blanket-type-ignore + + - repo: https://github.com/pre-commit/mirrors-mypy - rev: v1.10.0 + rev: v1.13.0 hooks: - id: mypy additional_dependencies: @@ -71,17 +79,23 @@ repos: - types-mock - types-six - types-toml + - werkzeug - repo: https://github.com/rcmdnk/pyproject-pre-commit - rev: v0.1.9 + rev: v0.3.4 hooks: - id: shellcheck - - id: mdformat-check - exclude: README-Unicode.md - language: python + + +- repo: https://github.com/igorshubovych/markdownlint-cli + rev: v0.42.0 + hooks: + - id: markdownlint + + - repo: https://github.com/pycqa/pylint - rev: v2.17.4 + rev: v3.3.1 hooks: - id: pylint args: @@ -89,51 +103,32 @@ repos: -sn, # Don't display the score --load-plugins=pylint.extensions.eq_without_hash, --ignore-imports=yes, - "--disable=duplicate-code,line-too-long", + "--disable=duplicate-code,line-too-long,import-error,no-name-in-module,c-extension-no-member", ] log_file: ".git/pre-commit-pylint.log" additional_dependencies: - - pyfakefs - six - mock - pandas - pytest_forked - toml + + - repo: local hooks: - - id: run-pyre - name: run-pyre (expect this to take 30 seconds) - entry: python pyre_runner.py - types: [python] - language: python - log_file: ".git/pre-commit-pyre.log" - additional_dependencies: [pyre-check,mock] - - id: pytype - name: pytype (may take up to two minutes) - entry: sh -c "pytype >/dev/tty" - types: [python] - verbose: true - language: python - language_version: python3.8 - require_serial: true - additional_dependencies: [pytype] - id: pytest name: Check pytest unit tests pass types: [python] # entry: sh -c "pytest -x -rf --new-first --show-capture=all >/dev/tty" - entry: sh -c "tox -e py38-covcombine >/dev/tty" + entry: sh -c "tox >/dev/tty" verbose: true language: python require_serial: true pass_filenames: false -- repo: https://github.com/pre-commit/pygrep-hooks - rev: v1.10.0 # Use the ref you want to point at - hooks: - # Enforce that `# type: ignore` annotations always occur with specific codes. - # Sample annotations: # type: ignore[attr-defined] # type: ignore[attr-defined,name-defined] - - id: python-check-blanket-type-ignore + + - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.6.0 + rev: v5.0.0 hooks: - id: trailing-whitespace - id: end-of-file-fixer diff --git a/.pylintrc b/.pylintrc index dc41d223..5b5816c0 100644 --- a/.pylintrc +++ b/.pylintrc @@ -65,6 +65,7 @@ disable=W0142,W0703,C0111,R0201,W0603,W0613,W0212,W0141, len-as-condition, no-else-return, raise-missing-from, + too-many-positional-arguments, too-many-branches, too-many-nested-blocks, too-many-statements, @@ -222,8 +223,9 @@ defining-attr-methods=__init__,__new__,setUp [DESIGN] -# Maximum number of arguments for function / method -max-args=100 +# Maximum number of arguments for function / method. +# defaults to: max-args=5 +max-args=10 # Argument names that match this expression will be ignored. Default to name # with leading underscore diff --git a/.vscode/ltex.dictionary.en-US.txt b/.vscode/ltex.dictionary.en-US.txt index f92c872c..0804ae9d 100644 --- a/.vscode/ltex.dictionary.en-US.txt +++ b/.vscode/ltex.dictionary.en-US.txt @@ -23,3 +23,5 @@ tokenless virtualenv virutalenv workdir +xapi +xensource diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 910602de..f50406ac 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,3 +1,5 @@ +# Development setup + ## Development setup on Fedora 37 On Fedora 37, the `tox` rpm installs all Python versions. @@ -7,8 +9,24 @@ But this `tox` is older, so install `tox==4.5.1` using `pip` (see below) sudo dnf install tox;sudo rpm -e tox ``` +## Development setup on Ubuntu 24.04 + +Use `sudo apt install pipx` to install test tools like `pytest`, `pylint` and `tox`: + +```bash +sudo apt install pipx +pipx install tox; pipx install 'pytest<7';pipx install pylint +pipx inject pytest pytest-{forked,localftpserver,pythonpath,subprocess,timeout} pyfakefs pytest_httpserver six mock +pipx inject pylint pyfakefs six mock pytest{,_forked,-localftpserver} +``` + +Use the `deadsnakes` ppa to install Python versions like 3.8 and 3.11 (see below) + ## Development setup on Ubuntu 22.04 +Usage of to install +other python versions. + ```bash sudo apt update sudo apt install software-properties-common python{2,3}-dev @@ -19,15 +37,18 @@ sudo apt-get install -y python3.{8,11}{,-distutils} Installation of additional python versions for testing different versions: - If `deadsnakes/ppa` does not work, e.g. for Python 3.6, `conda` or `pyenv` can be used. - For instructions, see https://realpython.com/intro-to-pyenv: - ```yaml + For instructions, see : + + ```bash sudo apt install -y build-essential xz-utils zlib1g-dev \ lib{bz2,ffi,lzma,readline,ssl,sqlite3}-dev curl https://pyenv.run | bash # add displayed commands to .bashrc ~/.pyenv/bin/pyenv install 3.{6,8,11} && ~/.pyenv/bin/pyenv local 3.{6,8,11} # builds them ``` + - For testing on newer Ubuntu which has `python2-dev`, but not `pip2`, install `pip2` this way: - ```yml + + ```bash curl https://bootstrap.pypa.io/pip/2.7/get-pip.py --output get-pip.py;sudo python2 get-pip.py ``` @@ -48,15 +69,15 @@ This project uses `tox` to run the tests for different python versions. Intro: > _"Managing a Project's Virtual environments with `tox` - > A comprehensive beginner's introduction to `tox`":_ -> https://www.seanh.cc/2018/09/01/tox-tutorial/ +> `tox` runs `pytest`, `pylint` and static analysis using `mypy`, `pyre`, `pytype`, and `pyright`. Links: -- https://mypy.readthedocs.io/en/stable/ -- https://microsoft.github.io/pyright/ -- https://google.github.io/pytype/ -- https://pyre-check.org/ +- +- +- +- With `tox`, developers can run the full test suite for Python 2.7 and 3.x. The same test suite is used in GitHub CI: @@ -113,13 +134,15 @@ pip install "pytest<7" pytest-picked pytest-sugar pytest-clarity # pytest-icdiff ``` To verify or extract the dependencies and extras configured in `pyproject.toml` and `tox.ini` -for specific `tox` environments, you can use https://pypi.org/project/tox-current-env/: +for specific `tox` environments, you can use +: ```bash tox --print-deps-to=pytype-deps.txt --print-extras-to=pytype-extras.txt -e pytype ``` -For more information to debug `pytest` test suites see: https://stribny.name/blog/pytest/ +For more information to debug `pytest` test suites see +: ## Running GitHub actions locally using `act` @@ -177,7 +200,7 @@ docker run -it --rm alpine:latest grep NAME /etc/os-release Ubuntu 22.04 LTS uses `iptables-nft` by default. Switch to `iptables-legacy` so that Docker will work: -https://crapts.org/2022/05/15/install-docker-in-wsl2-with-ubuntu-22-04-lts/ + ### Copy selection on selecting test (without need for Ctrl-C) @@ -186,11 +209,11 @@ to the X selection/clipboard for pasting it. To use this engrained behavior on Windows as well, it seems the only reliable way to have it for all apps is a `AutoHotKey` script: -- https://www.ilovefreesoftware.com/30/tutorial/auto-copy-selected-text-clipboard-windows-10.html +- While individual extensions for VS Code, Firefox, chrome do work partially, they either don't cover the Firefox URL bar, the VS Code terminal and so on: -- https://addons.mozilla.org/en-GB/firefox/addon/copy-on-select -- https://marketplace.visualstudio.com/items?itemName=dinhani.copy-on-select (VS Code) -- https://www.jackofalladmins.com/knowledge%20bombs/dev%20dungeon/windows-terminal-copy-selection/ +- +- (VS Code) +- diff --git a/README-Unicode.md b/README-Unicode.md index 82e0999f..5c970810 100644 --- a/README-Unicode.md +++ b/README-Unicode.md @@ -1,40 +1,64 @@ -Python3 Unicode migration in the XCP package -============================================ +# Python3 Unicode migration in the XCP package + +## Problem + +Python3.6 on XS8 does not have an all-encompassing default UTF-8 mode for I/O. + +Newer Python versions have an UTF-8 mode that they even enable by default. +Python3.6 only enabled UTF-8 for I/O when an UTF-8 locale is used. +See below for more background info on the UTF-8 mode. + +For situations where UTF-8 enabled, we have to specify UTF-8 explicitly. + +Such sitation happens when LANG or LC_* variables are not set for UTF-8. +XAPI plugins like auto-cert-kit find themself in this situation. + +Example: +For reading UTF-8 files like the `pciids` file, add `encoding="utf-8"`. +This applies especailly to `open()` and `Popen()` when files my contain UTF-8. + +This also applies when en/decoding to/form `urllib` which uses bytes. +`urllib` has to use bytes as HTTP data can of course also be binary, e.g. compressed. + +## Migrating `subprocess.Popen()` -Migrating `subprocess.Popen()` ------------------------------- With Python3, the `stdin`, `stdout` and `stderr` pipes for `Popen()` default to `bytes`(binary mode). Binary mode is much safer because it foregoes the encode/decode. The for working with strings, existing users need to either enable text mode (when safe, it will attempt to decode and encode!) or be able to use bytes instead. For cases where the data is guaranteed to be pure ASCII, such as when resting the `proc.stdout` of `lspci -nm`, it is sufficient to use: + ```py open(["lspci, "-nm"], stdout=subprocess.PIPE, universal_newlines=True) ``` + This is possible because `universal_newlines=True` is accepted by Python2 and Python3. On Python3, it also enables text mode for `subprocess.PIPE` streams (newline conversion not needed, but text mode is needed) -Migrating `builtins.open()` ---------------------------- +## Migrating `builtins.open()` + On Python3, `builtins.open()` can be used in a number of modes: + - Binary mode (when `"b"` is in `mode`): `read()` and `write()` use `bytes`. - Text mode (Python3 default up to Python 3.6), when UTF-8 character encoding is not set by the locale -- UTF-8 mode (default since Python 3.7): https://peps.python.org/pep-0540/ +- UTF-8 mode (default since Python 3.7): When no Unicode locale in force, like in XAPI plugins, Python3 will be in text mode or UTF-8 (since Python 3.7, but existing XS is on 3.6): -* By default, `read()` on files `open()`ed without selecting binary mode attempts +- By default, `read()` on files `open()`ed without selecting binary mode attempts to decode the data into the Python3 Unicode string type. This fails for binary data. Any `ord() >= 128`, when no UTF-8 locale is active With Python 3.6, triggers `UnicodeDecodeError`. -* Thus, all `open()` calls which might open binary files have to be converted to binary +- Thus, all `open()` calls which might open binary files have to be converted to binary or UTF-8 mode unless the caller is sure he is opening an ASCII file. But even then, enabling an error handler to handle decoding errors is recommended: + ```py open(filename, errors="replace") ``` + But neither `errors=` nor `encoding=` is accepted by Python2, so a wrapper is likely best. ### Binary mode @@ -45,7 +69,7 @@ However, when strings need to returned from the library, something like `bytes.d ### Text mode -Text mode using the `ascii` codec should be only used when it is ensured that the data can only consist of ASCII characters (0-127). Sadly, it is the default in Python 3.6 when the Python interpreter was not started using an UTF-8 locale for the LC_CTYPE locale category (set by LC_ALL, LC_CTYPE, LANG environment variables, overriding each other in that order) +Text mode using the `ascii` codec should be only used when it is ensured that the data can only consist of ASCII characters (0-127). Sadly, it is the default in Python 3.6 when the Python interpreter was not started using an UTF-8 locale for the LC_CTYPE locale category (set by LC_ALL, LC_CTYPE, LANG environment variables, overriding each other in that order) ### UTF-8 mode @@ -54,10 +78,11 @@ Most if the time, the `UTF-8` codec should be used since even simple text files Some files or some output data from commands even contains legacy `ISO-8859-1` chars, and even the `UTF-8` codec would raise `UnicodeDecodeError` for these. When this is known to be the case, `encoding="iso-8859-1` could be tried (not tested yet). -### Problems +### Problems With the locale set to C (XAPI plugins have that), Python's default mode changes between 3.6 and 3.7: + ```py for i in 3.{6,7,10,11};do echo -n "3.$i: "; LC_ALL=C python3.$i -c 'import locale,sys;print(locale.getpreferredencoding())';done @@ -66,7 +91,9 @@ for i in 3.{6,7,10,11};do echo -n "3.$i: "; 3.10: UTF-8 3.11: utf-8 ``` + This has the effect that in Python 3.6, the default codec for XAPI plugins is `ascii`: + ```py for i in 2.7 3.{6,7};do echo "$i:"; LC_ALL=C python$i -c 'open("/usr/share/hwdata/pci.ids").read()';done @@ -79,6 +106,7 @@ Traceback (most recent call last): UnicodeDecodeError: 'ascii' codec can't decode byte 0xc2 in position 97850: ordinal not in range(128) 3.7: ``` + This error means that the `'ascii' codec` cannot handle input ord() >= 128, and as some Video cards use `²` to reference their power, the `ascii` codec chokes on them. It means `xcp.pci.PCIIds()` cannot use `open("/usr/share/hwdata/pci.ids").read()`. @@ -86,6 +114,7 @@ It means `xcp.pci.PCIIds()` cannot use `open("/usr/share/hwdata/pci.ids").read() While Python 3.7 and newer use UTF-8 mode by default, it does not set up an error handler for `UnicodeDecodeError`. As it happens, some older tools output ISO-8859-1 characters hard-coded and these aren't valid UTF-8 sequences, and even newer Python versions need error handlers to not fail: + ```py echo -e "\0262" # ISO-8859-1 for: "²" python3 -c 'open(".text").read()' @@ -133,6 +162,7 @@ tests/test_bootloader.py line 38 in TestLinuxBootloader.setUp() tests/test_pci.py line 96 in TestPCIIds.test_videoclass_by_mock_calls() tests/test_pci.py line 110 in TestPCIIds.mock_lspci_using_open_testfile() ``` + Of course, `xcp/net/ifrename` won't be affected but it would be good to fix the warning for them as well in an intelligent way. See the proposal for that below. @@ -141,6 +171,7 @@ arguments we need to pass to ensure that all users of open() will work, we need to make passing the arguments conditional on Python >= 3. 1. Overriding `open()`, while technically working would not only affect xcp.python but the entire program: + ```py if sys.version_info >= (3, 0): original_open = __builtins__["open"] @@ -152,7 +183,9 @@ to make passing the arguments conditional on Python >= 3. return original_open(*args, **kwargs) __builtins__["open"] = uopen ``` + 2. This is sufficient but is not very nice: + ```py # xcp/utf8mode.py if sys.version_info >= (3, 0): @@ -165,9 +198,11 @@ to make passing the arguments conditional on Python >= 3. - open(filename) + open(filename, **open_utf8args) ``` + But, `pylint` will still warn about these lines, so I propose: 3. Instead, use a wrapper function, which will also silence the `pylint` warnings at the locations which have been changed to use it: + ```py # xcp/utf8mode.py if sys.version_info >= (3, 0): @@ -189,4 +224,4 @@ Using the 3rd option, the `pylint` warnings for the changed locations explicitly disabling them. PS: Since utf8open() still returns a context-manager, `with open(...) as f:` -would still work. \ No newline at end of file +would still work. diff --git a/README.md b/README.md index 2f24022a..82c000b3 100644 --- a/README.md +++ b/README.md @@ -1,10 +1,10 @@ +# Common XenServer/XCP-ng Python classes + [![pre-commit](https://img.shields.io/badge/pre--commit-enabled-brightgreen?logo=pre-commit)](https://github.com/pre-commit/pre-commit) [![](https://img.shields.io/badge/python-2.7_%7C_3.6_%7C_3.7_%7C_3.8_%7C_3.9_%7C_3.10_%7C_3.11+-blue.svg)](https://www.python.org/downloads/) [![codecov](https://codecov.io/gh/xenserver/python-libs/branch/master/graph/badge.svg?token=6WKVLDXJFN)](https://codecov.io/gh/xenserver/python-libs) [![](https://img.shields.io/badge/License-BSD--2--Cause%20%26%20MIT-brightgreen)](https://github.com/xenserver/python-libs/blob/master/LICENSE) -# Common XenServer/XCP-ng Python classes - The `xcp` directory contains the Common XenServer and XCP-ng Python packages. They are intented for use in XenServer and XCP-ng Dom0 only and deal with logging, Hardware/PCI, networking, and other Dom0 tasks. @@ -18,6 +18,8 @@ It depends on `six`, and on Python 2.7, also `configparser` and `pyliblzma`. ## Test-driven Development (TDD) Model +Please see [CONTRIBUTING.md] for installing a local development environment. + This package has CI which can be run locally but is also run in GitHub CI to ensure Test-driven development. @@ -82,7 +84,7 @@ For the installation of the general development dependencies, visit [INSTALL.md] The list of `virtualenvs` configured in tox can be shown using this command: `tox -av` -```yaml +```ml $ tox -av default environments: py36-lint -> Run in a py36 virtualenv: Run pylint and fail on warnings remaining on lines in the diff to master @@ -115,17 +117,19 @@ The goal or final benefit would be to have it to ensure internal type correctnes and code quality but also to use static analysis to check the interoperability with the calling code. -## Type annotations: Use Type comments for now! +## Type annotations: Use Type comments for now Python2.7 can't support the type annotation syntax, but until all users are migrated, annotations in comments (type comments) can be used. They are supported by tools like `mypy` and `pyright` (VS Code): -Quoting from https://stackoverflow.com/questions/53306458/python-3-type-hints-in-python-2: +Quoting from : > Function annotations were introduced in [PEP 3107](https://www.python.org/dev/peps/pep-3107/) for Python 3.0. The usage of annotations as type hints was formalized in in [PEP 484](https://www.python.org/dev/peps/pep-0484/) for Python 3.5+. > -> Any version before 3.0 then will not support the syntax you are using for type hints at all. However, PEP 484 [offers a workaround](https://www.python.org/dev/peps/pep-0484/#suggested-syntax-for-python-2-7-and-straddling-code), which some editors may choose to honor. In your case, the hints would look like this: +> Python < 3.0 do support the type hints syntax. +> However, PEP 484 [offers a workaround](https://www.python.org/dev/peps/pep-0484/#suggested-syntax-for-python-2-7-and-straddling-code) + These type comment look like this: ```py def get_default_device(use_gpu=True): @@ -171,30 +175,19 @@ xcp/xmlunwrap.py Completed in 0.604sec ``` -See https://github.com/xenserver/python-libs/pull/23 for the context of this example. +See for the context of this example. -## Special open TODOs: +## Guidelines Charset encoding/string handling: -See [README-Unicode.md](README-Unicode.md) for details: - -- What's more: When code is called from a xapi plugin (such as ACK), when such code - attempts to read text files like the `pciids` file, and there is a Unicode char - it int, and the locale is not set up to be UTF-8 (because xapi plugins are started - from xapi), the UTF-8 decoder has to be explicitly enabled for these files, - bese by adding `encoding="utf-8"` to the arguments of these specific `open()` calls, - to have valid Unicode text strings, e.g. `xcp.pci`, for regular text processing. -- TODO: More to be opened for all remaining `open()` and `Popen()` users, - as well as ensuring that users of `urllib` are able to work with they bytes - it returns (there is no option to use text mode, data may be gzip-encoded!) +See [README-Unicode.md](README-Unicode.md) for details on Unicode support. ## Users -- https://github.com/xenserver/host-installer - - /opt/xensource/installer/ (has copies of `cpiofile.py`, `repository.py` (with `accessor.py`) -- https://github.com/xcp-ng-rpms/host-upgrade-plugin ([koji](https://koji.xcp-ng.org/packageinfo?packageID=104)): +- [host-installer](https://github.com/xenserver/host-installer) +- [host-upgrade-plugin](https://github.com/xcp-ng-rpms/host-upgrade-plugin) ([koji](https://koji.xcp-ng.org/packageinfo?packageID=104)): - /etc/xapi.d/plugins/prepare_host_upgrade.py -- https://github.com/xapi-project/xen-api (`xapi-core.rpm` and `xenopsd.rpm`) +- [xapi](https://github.com/xapi-project/xen-api) (`xapi-core.rpm` and `xenopsd.rpm`) - /etc/xapi.d/extensions/pool_update.apply - /etc/xapi.d/extensions/pool_update.precheck - /etc/xapi.d/plugins/disk-space @@ -207,16 +200,16 @@ See [README-Unicode.md](README-Unicode.md) for details: - xenserver-release-config/[xcp-ng-release-config](https://koji.xcp-ng.org/rpminfo?rpmID=10250) - /opt/xensource/libexec/fcoe_driver - /opt/xensource/libexec/xen-cmdline -- https://github.com/xcp-ng-rpms/interface-rename: +- - /etc/sysconfig/network-scripts/interface-rename.py - /opt/xensource/bin/interface-rename - pvsproxy (Proprietary) - /usr/libexec/xapi-storage-script/volume/org.xen.xapi.storage.tmpfs/memoryhelper.py -- https://github.com/xenserver/linux-guest-loader (not installed by default anymore) +- (not installed by default anymore) - /opt/xensource/libexec/eliloader.py -- https://github.com/xcp-ng-rpms/vcputune +- - /opt/xensource/bin/host-cpu-tune -- The ACK xenapi plugin see: https://github.com/xenserver/python-libs/pull/21 +- The ACK xapi plugin. See: Verification: diff --git a/pyproject.toml b/pyproject.toml index 474a6e80..fee467da 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -71,11 +71,6 @@ mypy = [ "types-six", "types-toml", ] -# pyre introduced two false-postives recently, pin it to prevent further surprises: -pyre = [ - "pyre-check == 0.9.21", - "pyre-extensions == 0.0.30", -] pytype = [ "pandas", "pytype", @@ -116,11 +111,12 @@ show_error_context = true error_summary = true files = ["xcp", "tests/test_*.py"] mypy_path = "stubs" -python_version = "3.10" +python_version = "3.11" warn_redundant_casts = true warn_return_any = true warn_unreachable = true warn_unused_configs = true +explicit_package_bases = true disallow_any_unimported = true disallow_any_explicit = false disallow_any_generics = true @@ -198,7 +194,7 @@ exclude = [ extraPaths = ["stubs"] include = ["xcp", "tests"] pythonPlatform = "Linux" -pythonVersion = "3.6" +pythonVersion = "3.11" reportFunctionMemberAccess = true reportGeneralTypeIssues = "warning" reportOptionalMemberAccess = "warning" @@ -211,7 +207,7 @@ stubPath = "stubs" inputs = ["xcp", "tests", "./*.py"] keepgoing = true platform = "linux" -python_version = "3.10" +python_version = "3.11" pythonpath = ".:stubs" disable = ["ignored-type-comment"] overriding_parameter_count_checks = true diff --git a/pyre_runner.py b/pyre_runner.py deleted file mode 100755 index a67d2427..00000000 --- a/pyre_runner.py +++ /dev/null @@ -1,62 +0,0 @@ -#!/usr/bin/env python -""" -Run a one-time pyre static analysis check without needing a .pyre_configuration -Gets the paths dynamically so it can be used in tox and GitHub CI -""" -import os -import sys -import time - -import mock - -me = os.path.basename(__file__) + ":" - -pyre_typesched = os.environ.get("PYRE_TYPESHED", None) -if pyre_typesched and os.path.exists(pyre_typesched + "/stdlib/os/path.pyi"): - print("Using {env:PYRE_TYPESHED}:", pyre_typesched) -else: - pyre_typesched = sys.path[-1] + "/mypy/typeshed" - if os.path.exists(pyre_typesched + "/stdlib/os/path.pyi"): - print("Using python_lib:", pyre_typesched) - else: - pyre_typesched = "/tmp/typeshed" - if os.path.exists(pyre_typesched + "/stdlib/os/path.pyi"): - print("Using:", pyre_typesched) - else: - clone = "git clone --depth 1 https://github.com/python/typeshed " - print(me, "Falling back to:", clone + pyre_typesched) - ret = os.system(clone + pyre_typesched) - if ret or not os.path.exists(pyre_typesched + "/stdlib/os/path.pyi"): - print(me, "Could not find or clone typeshed, giving up.") - sys.exit(0) - -command = ( - "pyre", - "--source-directory", - "xcp", - "--source-directory", - "tests", - "--search-path", - "stubs", - "--search-path", - ".", - "--search-path", - os.path.dirname(mock.__file__), - "--typeshed", - pyre_typesched, - "check", -) -cmd = " ".join(command) -print(me, "Running:", cmd) -start_time = time.time() -ret = os.system(cmd) -duration = time.time() - start_time -r = os.waitstatus_to_exitcode(ret) # type: ignore[module-addr] # newer versions have it -if r == 0: - print(me, f"OK pyre took: {duration:.1f}s") -else: - print(me, "Ran:", cmd) - print(me, "exit code:", r) - if os.environ.get("ACT", None): - time.sleep(10) -sys.exit(r) diff --git a/pytest.ini b/pytest.ini index e07456ee..4a1c8466 100644 --- a/pytest.ini +++ b/pytest.ini @@ -11,7 +11,6 @@ required_plugins = pytest_httpserver pytest-forked - pytest-localftpserver pytest-pythonpath pytest-subprocess pytest-timeout @@ -38,4 +37,5 @@ filterwarnings=ignore:Unknown config option pythonpath=stubs # Disable when using pytest >= 7.0.0: # Used by pytest-pythonpath for Python >=2.7 (https://pypi.org/project/pytest-pythonpath): -python_paths=stubs +# Can now only be activated when using newer pytest: +#python_paths=stubs diff --git a/pytype_runner.py b/pytype_runner.py index 3ee62e98..9625736f 100755 --- a/pytype_runner.py +++ b/pytype_runner.py @@ -77,7 +77,7 @@ def run_pytype(command: List[str], branch_url: str, errorlog: TextIO, results): ok = True while ok: for key, _ in sel.select(): - line = key.fileobj.readline() # type: ignore + line = key.fileobj.readline() # type: ignore[union-attr] if not line: ok = False break diff --git a/stubs/pyfakefs/__init__.pyi b/stubs/pyfakefs/__init__.pyi deleted file mode 100644 index e69de29b..00000000 diff --git a/stubs/pyfakefs/fake_filesystem_unittest.pyi b/stubs/pyfakefs/fake_filesystem_unittest.pyi deleted file mode 100644 index aade18b0..00000000 --- a/stubs/pyfakefs/fake_filesystem_unittest.pyi +++ /dev/null @@ -1,20 +0,0 @@ -from types import ModuleType -from typing import Any, Callable, Dict, List, Optional, Union - -from _typeshed import Incomplete # pylint: disable=import-error - -Patcher = Incomplete -PatchMode = Incomplete - -def patchfs( - _func: Optional[Incomplete] = ..., - *, - additional_skip_names: Optional[List[Union[str, ModuleType]]] = ..., - modules_to_reload: Optional[List[ModuleType]] = ..., - modules_to_patch: Optional[Dict[str, ModuleType]] = ..., - allow_root_user: bool = ..., - use_known_patches: bool = ..., - patch_open_code: PatchMode = ..., - patch_default_args: bool = ..., - use_cache: bool = ..., -) -> Callable[[Any], Any]: ... diff --git a/stubs/pytest.pyi b/stubs/pytest.pyi deleted file mode 100644 index 6e31256b..00000000 --- a/stubs/pytest.pyi +++ /dev/null @@ -1,8 +0,0 @@ -# pylint: disable=reimported,no-name-in-module,unused-import,function-redefined.redefined-builtin -from _pytest.python_api import raises -from _typeshed import Incomplete as fixture -from _typeshed import Incomplete as mark - -def skip(msg: str = "", *, allow_module_level: bool = False): ... - -__all__ = ["mark", "fixture", "skip", "raises"] diff --git a/stubs/werkzeug/wrappers.pyi b/stubs/werkzeug/wrappers.pyi deleted file mode 100644 index f4a46d17..00000000 --- a/stubs/werkzeug/wrappers.pyi +++ /dev/null @@ -1,3 +0,0 @@ -from _typeshed import Incomplete # pylint: disable=import-error,no-name-in-module -Request = Incomplete -Response = Incomplete diff --git a/tests/conftest.py b/tests/conftest.py index 1a2a4974..68cc4fd5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,10 +5,20 @@ This module is run automatically by pytest to define and enable fixtures. """ -# pyre-ignore-all-errors[21] +import os +import tempfile import warnings -import pytest # pyre does not find the module when run by tox -e py311-pyre +import pytest + + +@pytest.fixture() +def mount_dir(): + """pytest fixture for getting a temporary name for a temp directory""" + mount_point = tempfile.mkdtemp(prefix="media-", dir="/tmp") + os.rmdir(mount_point) + return mount_point.encode() + @pytest.fixture(autouse=True) def set_warnings(): diff --git a/tests/httpserver_testcase.py b/tests/httpserver_testcase.py index b9dc5ceb..cad59524 100644 --- a/tests/httpserver_testcase.py +++ b/tests/httpserver_testcase.py @@ -1,4 +1,5 @@ import os +import sys import unittest from typing import Callable, Optional @@ -11,9 +12,10 @@ from pytest_httpserver import HTTPServer from werkzeug.wrappers import Request, Response - ErrorHandler = Optional[Callable[[Request], Response]] + ErrorHandler = Optional[Callable[[Request], Response | None]] except ImportError: pytest.skip(allow_module_level=True) + sys.exit(0) # Let pyright know that this is a dead end class HTTPServerTestCase(unittest.TestCase): @@ -31,7 +33,7 @@ def tearDownClass(cls): @classmethod def serve_file(cls, root, file_path, error_handler=None, real_path=None): - # type:(str, str, Optional[Callable[[Request], Response]], Optional[str]) -> None + # type:(str, str, ErrorHandler, Optional[str]) -> None """Expect a GET request and handle it using the local pytest_httpserver.HTTPServer""" def handle_get(request): diff --git a/tests/test_accessor.py b/tests/test_accessor.py index 93f5d609..aae6b60a 100644 --- a/tests/test_accessor.py +++ b/tests/test_accessor.py @@ -1,24 +1,27 @@ import unittest - -from pyfakefs.fake_filesystem import FakeFilesystem +from typing import TYPE_CHECKING import xcp.accessor from .test_mountingaccessor import check_binary_read, check_binary_write +if TYPE_CHECKING: + import pyfakefs + import pytest + -def test_file_accessor(fs): - # type:(FakeFilesystem) -> None +def test_file_accessor(fs, caplog): + # type(pyfakefs.fake_filesystem.FakeFilesystem, pytest.LogCaptureFixture) -> None """Test FileAccessor.writeFile(), .openAddress and .access using pyfakefs""" accessor = xcp.accessor.createAccessor("file://repo/", False) assert isinstance(accessor, xcp.accessor.FileAccessor) check_binary_read(accessor, "/repo", fs) - check_binary_write(accessor, "/repo", fs) + check_binary_write(accessor, "/repo", fs, caplog) class TestAccessor(unittest.TestCase): def setUp(self): - """Provide the refrence content of the repo/.treeinfo file for check_repo_access()""" + """Provide the reference content of the repo/.treeinfo file for check_repo_access()""" with open("tests/data/repo/.treeinfo", "rb") as dot_treeinfo: self.reference_treeinfo = dot_treeinfo.read() @@ -35,6 +38,7 @@ def check_repo_access(self, a): self.assertFalse(a.access('no_such_file')) self.assertEqual(a.lastError, 404) a.finish() + a.finish() # Test and cover already finished accessors def test_filesystem_accessor_access(self): """Test FilesystemAccessor.access()""" diff --git a/tests/test_bootloader.py b/tests/test_bootloader.py index 82500336..23b21a5e 100644 --- a/tests/test_bootloader.py +++ b/tests/test_bootloader.py @@ -41,8 +41,7 @@ def test_grub2(self): universal_newlines=True) assert proc.stdout for line in proc.stdout: - # pylint: disable-next=deprecated-method - self.assertRegexpMatches(line, r"^(5a6,13$|>)") + self.assertRegex(line, r"^(5a6,13$|>)") # Replaced removed assert call proc.stdout.close() proc.wait() diff --git a/tests/test_ftpaccessor.py b/tests/test_ftpaccessor.py index a5b0b303..340b759c 100644 --- a/tests/test_ftpaccessor.py +++ b/tests/test_ftpaccessor.py @@ -21,13 +21,14 @@ from io import BytesIO import pytest -import pytest_localftpserver # pylint: disable=unused-import # Ensure that it is installed +import pytest_localftpserver # Ensure that it is installed from six import ensure_binary, ensure_str import xcp.accessor binary_data = b"\x80\x91\xaa\xb0\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xcc\xdd\xee\xff" text_data = "✋➔Hello Accessor from the 🗺, download and verify ✅ me!" +assert pytest_localftpserver def upload_textfile(ftpserver, accessor): diff --git a/tests/test_httpaccessor.py b/tests/test_httpaccessor.py index d0248f7f..c6bd38fa 100644 --- a/tests/test_httpaccessor.py +++ b/tests/test_httpaccessor.py @@ -43,7 +43,6 @@ 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): diff --git a/tests/test_ifrename_logic.py b/tests/test_ifrename_logic.py index 5d445f3c..f6966054 100644 --- a/tests/test_ifrename_logic.py +++ b/tests/test_ifrename_logic.py @@ -1,4 +1,4 @@ -# pyright: reportGeneralTypeIssues=false +# pyright: reportGeneralTypeIssues=false,reportAttributeAccessIssue=false # pytype: disable=attribute-error from __future__ import print_function from __future__ import unicode_literals diff --git a/tests/test_logger.py b/tests/test_logger.py index af8cd5c3..fb6b5a32 100644 --- a/tests/test_logger.py +++ b/tests/test_logger.py @@ -10,7 +10,8 @@ from xcp.logger import openLog -def test_openLog_mock_open(): +def test_openLog_mock_open(fs): + # type(FakeFilesystem) -> None """Cover xcp.logger.openLog.open_with_codec_handling and check the arguments used for open()""" fh = StringIO() with patch("xcp.compat.open", mock_open()) as open_mock: @@ -31,3 +32,4 @@ def test_openLog_mock_stdin(): assert openLog("test.log") is True os.close(slave_fd) os.close(master_fd) + open_mock.assert_called_once_with("test.log", "a", **open_utf8) diff --git a/tests/test_mountingaccessor.py b/tests/test_mountingaccessor.py index 9dea2d3f..f0f57641 100644 --- a/tests/test_mountingaccessor.py +++ b/tests/test_mountingaccessor.py @@ -1,8 +1,13 @@ """pytest tests testing subclasses of xcp.accessor.MountingAccessor using pyfakefs""" + +import logging +import os import sys from io import BytesIO from typing import TYPE_CHECKING, cast +import pytest +from pytest import LogCaptureFixture as LogCap from mock import patch from pyfakefs.fake_filesystem import FakeFileOpen, FakeFilesystem @@ -17,9 +22,8 @@ if TYPE_CHECKING: from typing_extensions import Literal else: - import pytest - pytest.skip(allow_module_level=True) + sys.exit(0) # Let pyright know that this is a dead end binary_data = b"\x00\x1b\x5b\x95\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xcc\xdd\xee\xff" @@ -28,8 +32,8 @@ def expect(fp, mount): fp.register_subprocess(mount) # type: ignore[arg-type] -def test_device_accessor(fs, fp): - # type: (FakeFilesystem, FakeProcess) -> None +def test_device_accessor(fs, fp, mount_dir, caplog): + # type: (FakeFilesystem, FakeProcess, bytes, LogCap) -> None assert isinstance(fp, FakeProcess) # Test xcp.mount.bindMount() @@ -37,15 +41,15 @@ def test_device_accessor(fs, fp): fp.register_subprocess(mount) # type: ignore[arg-type] assert xcp.mount.bindMount("src", "mountpoint_dest") is None - expect(fp, [b"/bin/mount", b"-t", b"iso9660", b"-o", b"ro", b"/dev/device", b"/tmp"]) - accessor = xcp.accessor.createAccessor("dev:///dev/device", False) + expect(fp, [b"/bin/mount", b"-t", b"iso9660", b"-o", b"ro", b"/dev/device", mount_dir]) + accessor = xcp.accessor.createAccessor("dev:///dev/device", [""]) assert isinstance(accessor, xcp.accessor.MountingAccessorTypes) - check_mounting_accessor(accessor, fs, fp) + check_mounting_accessor(accessor, fs, fp, mount_dir, caplog) -def test_nfs_accessor(fs, fp): - # type: (FakeFilesystem, FakeProcess) -> None +def test_nfs_accessor(fs, fp, mount_dir, caplog): + # type: (FakeFilesystem, FakeProcess, bytes, LogCap) -> None assert isinstance(fp, FakeProcess) mount = [ b"/bin/mount", @@ -54,21 +58,22 @@ def test_nfs_accessor(fs, fp): b"-o", b"tcp,timeo=100,retrans=1,retry=0", b"server/path", - b"/tmp", + mount_dir, ] expect(fp, mount) accessor = xcp.accessor.createAccessor("nfs://server/path", False) assert isinstance(accessor, xcp.accessor.NFSAccessor) - check_mounting_accessor(accessor, fs, fp) + check_mounting_accessor(accessor, fs, fp, mount_dir, caplog) -def check_mounting_accessor(accessor, fs, fp): - # type: (Literal[False] | xcp.accessor.Mount, FakeFilesystem, FakeProcess) -> None +def check_mounting_accessor(accessor, fs, fp, mount_point, caplog): + # type: (xcp.accessor.Mount, FakeFilesystem, FakeProcess, bytes, LogCap) -> None """Test subclasses of MountingAccessor (with xcp.cmd.runCmd in xcp.mount mocked)""" assert isinstance(accessor, xcp.accessor.MountingAccessorTypes) with patch("tempfile.mkdtemp") as tempfile_mkdtemp: - tempfile_mkdtemp.return_value = "/tmp" + os.mkdir(mount_point) + tempfile_mkdtemp.return_value = mount_point.decode() accessor.start() assert accessor.location @@ -80,13 +85,13 @@ def check_mounting_accessor(accessor, fs, fp): fs.add_mount_point(location) assert check_binary_read(accessor, location, fs) - assert check_binary_write(accessor, location, fs) + assert check_binary_write(accessor, location, fs, caplog) assert open_text(accessor, location, fs, UTF8TEXT_LITERAL) == UTF8TEXT_LITERAL if sys.version_info.major >= 3: fs.mount_points.pop(location) - umount = [b"/bin/umount", b"-d", b"/tmp"] + umount = [b"/bin/umount", b"-d", mount_point] fp.register_subprocess(umount) # type: ignore[arg-type] accessor.finish() @@ -116,15 +121,20 @@ def check_binary_read(accessor, location, fs): return cast(bytes, binary_file.read()) == binary_data -def check_binary_write(accessor, location, fs): - # type: (Literal[False] | xcp.accessor.AnyAccessor, str, FakeFilesystem) -> bool +def check_binary_write(accessor, location, fs, caplog): + # type: (xcp.accessor.AnyAccessor, str, FakeFilesystem, LogCap) -> bool """Test the writeFile() method of different types of local Accessor classes""" assert isinstance(accessor, xcp.accessor.LocalTypes) name = "binary_file_written_by_accessor" - accessor.writeFile(BytesIO(binary_data), name) - - assert accessor.access(name) + with caplog.at_level(logging.FATAL): + accessor.writeFile(BytesIO(binary_data), name) + if caplog.record_tuples: + logger, level, message = caplog.record_tuples[0] + assert logger == "root" + assert level == logging.FATAL + assert message == "Copying to " + name + assert accessor.access(name) with FakeFileOpen(fs, delete_on_close=True)(location + "/" + name, "rb") as written: return cast(bytes, written.read()) == binary_data diff --git a/tests/test_pci.py b/tests/test_pci.py index 1b723538..d3278507 100644 --- a/tests/test_pci.py +++ b/tests/test_pci.py @@ -11,6 +11,8 @@ if sys.version_info >= (3, 6): from pytest_subprocess.fake_process import FakeProcess +else: + pytest.skip(allow_module_level=True) class TestInvalid(unittest.TestCase): @@ -31,6 +33,7 @@ class TestValid(unittest.TestCase): def test_null_with_segment(self): + assert PCI.is_valid("0000:00:00.0") is True c = PCI("0000:00:00.0") self.assertEqual(c.segment, 0) @@ -43,6 +46,7 @@ def test_null_with_segment(self): def test_null_without_segment(self): + assert PCI.is_valid("00:00.0") is True c = PCI("00:00.0") self.assertEqual(c.segment, 0) @@ -54,6 +58,7 @@ def test_null_without_segment(self): def test_valid(self): + assert PCI.is_valid("8765:43:1f.3") is True c = PCI("8765:43:1f.3") self.assertEqual(c.segment, 0x8765) @@ -61,14 +66,31 @@ def test_valid(self): self.assertEqual(c.device, 0x1f) self.assertEqual(c.function, 0x3) + def test_valid_index(self): + + assert PCI.is_valid("8765:43:1f.3[0]") is True + assert PCI.is_valid("1234:56:01.7[1]") is True + c = PCI("1234:56:01.7[1]") + + self.assertEqual(c.segment, 0x1234) + self.assertEqual(c.bus, 0x56) + self.assertEqual(c.device, 0x01) + self.assertEqual(c.function, 0x7) + self.assertEqual(c.index, 0x1) + def test_equality(self): self.assertEqual(PCI("0000:00:00.0"), PCI("00:00.0")) + assert PCI("1234:56:01.7[1]") != PCI("1234:56:01.7[2]") + assert PCI("1234:56:01.2") >= PCI("1234:56:01.2") + assert PCI("1234:56:01.1") <= PCI("1234:56:01.2") + assert PCI("1234:56:01.3") > PCI("1234:56:01.2") + assert PCI("1234:56:01.1") < PCI("1234:56:02.2") if sys.version_info >= (2, 7): def assert_videoclass_devices(ids, devs): # type: (PCIIds, PCIDevices) -> None - """Verification function for checking the otuput of PCIDevices.findByClass()""" + """Verification function for checking the output of PCIDevices.findByClass()""" video_class = ids.lookupClass('Display controller') assert video_class == ["03"] sorted_devices = sorted(devs.findByClass(video_class), @@ -76,6 +98,7 @@ def assert_videoclass_devices(ids, devs): # type: (PCIIds, PCIDevices) -> None # Assert devs.findByClass() finding 3 GPUs from tests/data/lspci-mn in our mocked PCIIds DB: assert len(sorted_devices) == 3 + assert len(devs.findByClass("03", "80")) == 2 # For each of the found devices, assert these expected values: for (video_dev, @@ -164,6 +187,7 @@ def test_videoclass_by_mock_calls(fp): def mock_lspci_using_open_testfile(fp): """Mock xcp.pci.PCIDevices.Popen() using open(tests/data/lspci-mn)""" with open("tests/data/lspci-mn", "rb") as fake_data: - assert isinstance(fp, FakeProcess) + if sys.version_info >= (3, 6): + assert isinstance(fp, FakeProcess) fp.register_subprocess(["lspci", "-mn"], stdout=fake_data.read()) return PCIDevices() diff --git a/tox.ini b/tox.ini index d7386335..fbbefac5 100644 --- a/tox.ini +++ b/tox.ini @@ -1,11 +1,11 @@ [tox] # This is the order how tox runs the tests when used interactively during development. # Run the tests which uncover issues most often first! For example: -# 1. python 2.7 and 3.10 coverage test for changed, but not covered lines and mypy check +# 1. python 3.11 coveragest for changed, but not covered lines and mypy check # 2. python 3.6 test and pylint warnings from changed lines -# 3. pytype (needs Python 3.8 for best results) -# 4. pyre and pyright checks, pytest test report as markdown for GitHub Actions summary -envlist = py38-covcombine-check, py36-lint-test, py310-pytype, py311-pyre-mdreport +# 3. pytype (needs Python 3.8 or newer for best results) +# 4. pyright checks, pytest test report as markdown for GitHub Actions summary +envlist = py311-covcp-check-mdreport, py312-cov-pytype, py313-cov-lint-pyright isolated_build = true skip_missing_interpreters = true requires = @@ -28,9 +28,9 @@ commands = # https://github.com/actions/toolkit/blob/main/docs/commands.md#problem-matchers echo "::add-matcher::.github/workflows/PYTHONWARNINGS-problemMatcher.json" pytest --cov -v --new-first -x --show-capture=all -rA - sh -c 'ls -l {env:COVERAGE_FILE}' sh -c 'if [ -n "{env:PYTEST_MD_REPORT_OUTPUT}" -a -n "{env:GITHUB_STEP_SUMMARY}" ];then \ - sed -i "s/tests\(.*py\)/[&](&)/" {env:PYTEST_MD_REPORT_OUTPUT}; sed "/title/,/\/style/d" \ + mkdir -p $(dirname "{env:GITHUB_STEP_SUMMARY:.git/sum.md}"); \ + sed "s/tests\(.*py\)/[&](&)/" \ {env:PYTEST_MD_REPORT_OUTPUT} >{env:GITHUB_STEP_SUMMARY:.git/sum.md};fi' [testenv] @@ -42,14 +42,12 @@ description = Run in a {basepython} virtualenv: lint: {[lint]description} mdreport: Make a test report (which is shown in the GitHub Actions Summary Page) test: {[test]description} - # https://pypi.org/project/pyre-check/ pyre intro: https://youtu.be/0FSXS5kw2m4 - pyre: Run pyre for static analyis, only passes using: tox -e py311-pyre check: Run mypy for static analyis pytype: Run pytype for static analyis, intro: https://youtu.be/abvW0mOrDiY # checkers(mypy) need the pytest dependices as well: extras = {check,pytype}: {[check]extras} - {cov,covcp,covcombine,fox,check,lint,test,pytype,pyre,mdreport}: {[test]extras} + {cov,covcp,covcombine,fox,check,lint,test,pytype,pyright,mdreport}: {[test]extras} {cov,covcp,covcombine,fox}: {[cov]extras} deps = mdreport: pytest-md-report @@ -58,9 +56,7 @@ deps = {cov,covcp,covcombine,fox}: coverage[toml] {cov,covcp,covcombine,fox}: diff-cover {lint,fox}: {[lint]deps} - pyre: pyre-check - pyre: pyre-extensions - pyre: pyright + pyright: pyright pytype: {[pytype]deps} allowlist_externals = {cov,covcp,covcombine,fox,check,lint,test,mdreport}: echo @@ -81,7 +77,6 @@ passenv = covcp: HOME check: MYPY_FORCE_COLOR check: MYPY_FORCE_TERMINAL_WIDTH - pyre: PYRE_TYPESHED {fox,check,pytype}: TERM fox: DISPLAY fox: XAUTHORITY @@ -104,7 +99,7 @@ setenv = {[cov]setenv} commands = lint: {[lint]commands} - pyre: {[pyre]commands} + pyright: {[pyright]commands} check: {[check]commands} pytype: {[pytype]commands} {cov,covcp,covcombine,check,fox,test,mdreport}: {[test]commands} @@ -126,7 +121,7 @@ extras = coverage commands = coverage xml -o {envlogdir}/coverage.xml --fail-under {env:XCP_COV_MIN:68} coverage html -d {envlogdir}/htmlcov - coverage html -d {envlogdir}/htmlcov-tests --fail-under {env:TESTS_COV_MIN:96} \ + coverage html -d {envlogdir}/htmlcov-tests --fail-under {env:TESTS_COV_MIN:95} \ --include="tests/*" diff-cover --compare-branch=origin/master --include-untracked \ {env:PY3_DIFFCOVER_OPTIONS} --fail-under {env:DIFF_COV_MIN:92} \ @@ -189,6 +184,8 @@ python = 3.9: py39 3.10: py310 3.11: py311 + 3.12: py312 + 3.13: py313 [check] extras = mypy @@ -200,17 +197,16 @@ commands = ignore = W191,W293,W504,E101,E126,E127,E201,E202,E203,E221,E222,E226,E227,E241,E251,E261,E262,E265,E301,E302,E303,E305,E722,W391,E401,E402,E741 max-line-length = 129 -[pyre] +[pyright] commands = - pyre: python3.11 --version -V # Needs py311-pyre, does not work with py310-pyre - python pyre_runner.py - -pyright + pyright [pytype] deps = pytype pandas commands = - python3.10 -V # Needs python <= 3.10, and 3.10 is needed to parse new "|" syntax + python3 --version + # python3.11 -V # Needs python <= 3.10, and 3.10 is needed to parse new "|" syntax pytype --version # Runs pytype -j auto -k --config .github/workflows/pytype.cfg and parses the output: python3 pytype_runner.py # When switching versions, update .github/workflows/pytype.cfg too! diff --git a/xcp/accessor.py b/xcp/accessor.py index 9583dfe7..fa659a89 100644 --- a/xcp/accessor.py +++ b/xcp/accessor.py @@ -315,7 +315,8 @@ def finish(self): self.cleanup = False self.ftp = None - def access(self, path): # pylint: disable=arguments-differ,arguments-renamed + # pylint: disable-next=arguments-differ,arguments-renamed + def access(self, path): # pyright: ignore[reportIncompatibleMethodOverride] try: logger.debug("Testing "+path) self._cleanup() @@ -425,7 +426,7 @@ def __repr__(self): """Dict of supported accessors. The key is the URL scheme""" def createAccessor(baseAddress, *args): - # type: (str, bool | Tuple[bool, List[str]]) -> Literal[False] | AnyAccessor + # type: (str, bool | List[str]) -> Literal[False] | AnyAccessor """ Return instance of the appropriate Accessor subclass based on the baseAddress. @@ -442,7 +443,7 @@ def createAccessor(baseAddress, *args): Examples: accessor = createAccessor("http://example.com", True) - accessor = createAccessor("dev://example.com", True, ['iso9660', 'ext3']) + accessor = createAccessor("dev://example.com", ['iso9660', 'ext3']) if not accessor: fatal() else: diff --git a/xcp/bootloader.py b/xcp/bootloader.py index 353cc7ff..28eea30e 100644 --- a/xcp/bootloader.py +++ b/xcp/bootloader.py @@ -32,7 +32,7 @@ import xcp.cmd -# pyre-ignore-all-errors[21] +# pyre-ignore-all-errors[21,30] try: # xenserver-release.rpm puts a branding.py into our xcp installation directory: from xcp import branding # type:ignore[attr-defined] # pytype: disable=import-error except ImportError: # For CI, use stubs/branding.py (./stubs is added to pythonpath) diff --git a/xcp/logger.py b/xcp/logger.py index 19ed6de6..02995a05 100644 --- a/xcp/logger.py +++ b/xcp/logger.py @@ -64,9 +64,10 @@ def openLog(lfile, level=logging.INFO): h.close() handler = logging.handlers.RotatingFileHandler(lfile, maxBytes=2**31) + # pyre-ignore[16] old = fcntl.fcntl(handler.stream.fileno(), fcntl.F_GETFD) - fcntl.fcntl(handler.stream.fileno(), - fcntl.F_SETFD, old | fcntl.FD_CLOEXEC) + # pyre-ignore[16] + fcntl.fcntl(handler.stream.fileno(), fcntl.F_SETFD, old | fcntl.FD_CLOEXEC) # or if it is not a string, assume its a file-like object else: