Test driven py3, for merge, with full CI by bernhardkaindl · Pull Request #26 · xenserver/python-libs

bernhardkaindl · 2023-04-27T04:30:31Z

Update for the review: @pau:

I've pushed my I hope final big update now. It moves all the pytest and pylint into tox!

This reduced the amount of calls and the overhead extremely, it's really fast with tox!

The execution time is now 27s to 49s per job, so it's a no-brainer to run them all now, all run in parallel, I'm very happy and excited that I've tox made this possible!

Please review! I'll put the documentation into README.md, I hope this PR is ready then!

Afterwards I already have a number other PRs to fix the remaining Py3 issues in the queue.

Apply "Use of `unicode` needed to be immediately handled" by Yann. Reserved "but a few checks relying on `str` could become insufficient in python2 with the larger usage of unicode strings." for applying optionally these not-needed later, because they could change behavior and if needed, they would belong to these other commits. And, after 21 commits from Yann and my work on to of that, that didn't appear. Co-authored-by: Yann Dirson <yann.dirson@vates.fr> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

…conversion Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

…s to open() as ths is considered best practice. (cherry picked from cpython commit 6cef076ba5edbfa42239924951d8acbb087b3b19) Signed-off-by: Brett Cannon <bcannon@gmail.com>

…fication Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

…ated Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

Running tests on python3 did reveal some of them. Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

There is no guaranty about ordering of dict elements, and tests compare results derived from enumerating a dict element. We could have used an OrderedDict to store the formulae and get a predictible output order, but just considering the output as a set seems better. Only applying this to rules expected to hold more than one element. Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

Caught by extended test. Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

This goes away in python3. Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

hashlib came with python 2.5, and old md5 module disappears in 3.0 Originally by Yann Dirson, changed to not add .encode() for md5 creation, because encoding changes are dealt with in other commits. Co-authored-by: Yann Dirson <yann.dirson@vates.fr> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

Reported under python3 for members created on-the-fly in `setUp()` Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

With python3, pylint complains about `else: raise()` constructs. This rework avoids them and reduces cyclomatic complexity by using the error-out-first idiom. Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

This is supposed to be just a module renaming to conform to PEP8, see https://docs.python.org/3/whatsnew/3.0.html#library-changes The SafeConfigParser class has been renamed to ConfigParser in Python 3.2, and backported as addon package. The `readfp` method now triggers a deprecation warning to replace it with `read_file`. Originally authored by Yann Dirson. Co-authored-by: Yann Dirson <yann.dirson@vates.fr> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

xcp.xmlunwrap extracts XML Elements from XML, and for Python2, the unwrapped unicode is encoded into Py2:str(bytes). Python3 unwraps XML Text elements as the Py3:str type which is likewise Unicode, but since Py3:str is the native type, we don't want to encode the Py3:str to Py3:bytes as that would break the API for use on Python3. BEcause binary data is not legal XML content and XML Text elements are defined to be encoded text, UTF-8 is the standard encoding, which Python converts to. It this fine to only encode() to Py2:str(=bytes) on Python2 as a legacy operation which can be removed once we drop Python2. Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

.github/workflows/main.yml

codecov · 2023-04-27T14:04:40Z

Codecov Report

❗ No coverage uploaded for pull request base (master@efa489e). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head a288569 differs from pull request most recent head 349af90. Consider uploading reports for the commit 349af90 to get more accurate results

@@            Coverage Diff            @@
##             master      #26   +/-   ##
=========================================
  Coverage          ?   67.43%           
=========================================
  Files             ?       20           
  Lines             ?     3458           
  Branches          ?        0           
=========================================
  Hits              ?     2332           
  Misses            ?     1126           
  Partials          ?        0

Fix issue #19 based on the description and progress from PR xenserver#24. Allows for opening text and binary files in text and binary modes. Mode, encoding and error handling can be set by passing the parameters "encoding" and "errors" using the kwargs parameters from openAddress() and writeFile() to open(mode, **kwargs) and ftp.makefile(mode, **kwargs). Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

Because the upcoming commits enable pylint to be more accessible, fix the errors which would be shown around suggesting to pass encoding to open() calls. The previous patch changed the default for the mode parameter to "rb"/"wb" in order to fix reading from binary files with Python3. This is also conistent with HTTPAccessor only being able to read bytes because urllib cannot decompress HTTP gzip encodings on the fly, and a conversion from the decompressed data to unicode sting can only happen after decompression. Because of the need to default to binary mode, the pylint warning to pass encoding= to open() does not apply to these calls which are setup by their calling mode to use binary mode by default. The next commit will enforce binary mode unless encoding is specified. Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

The previous patch supresses the pylint warning that encoding= should be bassed to open calls because the default of open() is text mode, which means decoding on reads and encoding on writes. Until this commit, this means that it would be the responsibilty of each callers to specify an encoding and setup handling decoding errors when they want to use file-based accessors in text mode. To be safe against mis-use like `openAccess(file, mode="r")` (note: the mode parameter is new), we could ignore that request and enforce binary mode unless encoding was passed as well. Summary: Use bytes whenever possible as this avoids any errors around coding/conversion of bytes to Unicode strings and back to bytes. Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

Add and enable a parameterized test to read binary data from a file- and an HTTP based accessor. Originally authored by Yann Dirson. Extracted, refactored and squashed for a single considated and concise commit by me. Co-authored-by: Yann Dirson <yann.dirson@vates.fr> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

Comment from Yann: diff-cover defaults to origin/main in new version, it seems. Originally by Yann Dirson, Updates by me: - Fix the GitHub action deprecation warning caused by the use of old action repo version which use the deprecated node.js 12 - Rename the variable "pyversion" to the upstream "python-version" - Remove the installation of pyliblzma which is not used. - Add branding.py to fix GitHub CI with Python3 Co-authored-by: Yann Dirson <yann.dirson@vates.fr> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

Without these generated API descriptions, we'd have to disable all checks which use these APIs because we'd have to typing info for them. Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

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

Toxify the CI for not having to push to run CI, just run tox locally Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

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

branding.py

.github/workflows/main.yml

pyproject.toml

tox.ini

pyproject.toml

setup.py

bernhardkaindl · 2023-05-09T14:32:43Z

I reviewed the review comments a second time and commented/resolved them!

This PR has been used as the basis of #27 which as been merged to master now, closing as done!

…/add-pre-commit-checks Run pre-commit checks in CI (including the Xen-Bugtool Test Environment)

bernhardkaindl and others added 17 commits April 25, 2023 18:39

python3: use "six.ensure_binary" and "six.ensure_text" for str/bytes …

ac499f8

…conversion Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

Remove direct call's to file's constructor and replace them with call…

27c5396

…s to open() as ths is considered best practice. (cherry picked from cpython commit 6cef076ba5edbfa42239924951d8acbb087b3b19) Signed-off-by: Brett Cannon <bcannon@gmail.com>

python3: xcp.net.mac: use six.python_2_unicode_compatible for stringi…

ab23ef7

…fication Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

xcp.net.ifrename.logic: use "logger.warning", "logger.warn" is deprec…

4dbd2a2

…ated Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

python3: use raw strings for regexps, fixes insufficient quoting

7ebf6ff

Running tests on python3 did reveal some of them. Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

test_dom0: mock "open()" in a python3-compatible way

42728b3

Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

test_cpio: ensure paths are handled as text

b975afe

Caught by extended test. Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

cpiofile: migrate last "list.sort()" call still using a "cmp" argument

ff165f7

This goes away in python3. Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

Pylint complements: honor len-as-condition convention

e84c3a5

Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

Pylint complements: whitespace in expressions

26dff48

Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

Pylint complements: test_ifrename_logic: disable "no-member" warning

870ff05

Reported under python3 for members created on-the-fly in `setUp()` Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

Pylint complements: avoid no-else-raise "refactor" issues

32e5268

With python3, pylint complains about `else: raise()` constructs. This rework avoids them and reduces cyclomatic complexity by using the error-out-first idiom. Signed-off-by: Yann Dirson <yann.dirson@vates.fr>

bernhardkaindl force-pushed the test-driven-py3-merge branch 2 times, most recently from eea8531 to 1cb8193 Compare April 27, 2023 05:15

bernhardkaindl changed the title ~~test PR: Test driven py3 merge~~ Test driven py3, for merge, with full CI Apr 27, 2023

xenserver deleted a comment from codecov-commenter Apr 27, 2023

psafont reviewed Apr 27, 2023

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

psafont reviewed Apr 27, 2023

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

psafont reviewed Apr 27, 2023

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

psafont reviewed Apr 27, 2023

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

psafont reviewed Apr 27, 2023

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

psafont reviewed Apr 27, 2023

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

bernhardkaindl force-pushed the test-driven-py3-merge branch from 1cb8193 to d556e19 Compare April 27, 2023 14:03

bernhardkaindl force-pushed the test-driven-py3-merge branch from d556e19 to 252f791 Compare April 27, 2023 14:13

bernhardkaindl and others added 10 commits April 28, 2023 15:35

Add generated Python typing stubs to allow for better mypy checks

b51d179

Without these generated API descriptions, we'd have to disable all checks which use these APIs because we'd have to typing info for them. Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

accessor: fix pylint: funcs shall use matching argument names

1161e8e

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

accessor: apply trivial simplification

79d4b83

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

accessor: apply suggestion to join duplicated code

f7f1c3d

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

Add tox.ini, pyproject.toml & mypy. Add pylint to the GitHub Action view

24e7b0e

Toxify the CI for not having to push to run CI, just run tox locally Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

bernhardkaindl force-pushed the test-driven-py3-merge branch from 6e22f24 to 24e7b0e Compare April 28, 2023 16:32

bernhardkaindl requested a review from psafont April 28, 2023 16:44

bernhardkaindl force-pushed the test-driven-py3-merge branch from 046e848 to a288569 Compare April 29, 2023 10:32

bernhardkaindl added 2 commits April 29, 2023 15:56

tox.ini: py<ver>-cov-diff: Check pylint errors/warnings on changes lines

5f5658f

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

Add README.md and set the new package version to 3.0.0

349af90

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

bernhardkaindl force-pushed the test-driven-py3-merge branch from a288569 to 349af90 Compare April 29, 2023 13:58