Initial Python3 support with Test-Driven-Development enforced by CI (with git diff coverage and diff lint checking) + mypy, pyre and pytype #27

bernhardkaindl · 2023-05-04T15:07:25Z

I have worked on @ydirson's PR #17 and think that my work on it should be ready now.

With this PR, Python3 support is not 100% complete yet, but with all tests passing for Python3 and all non-whitespace changes covered by code coverage, it's a big leap forward.

Warnings and further bugs remain, but they have to be fixed in my other PRs!

The summary of reviews done so far is: It's fine, and I shall update the CodeCov upload to upload code coverage for Python2.7 and Python3 separately, but I can do this in a followup PR, no problem for now:

Initial Python3 support with Test-Driven-Development enforced by CI:

with git diff coverage and diff lint checking
with Codecov upload
with Static analysis using mypy, pyre and pytype

GitHub CI (but also local CI checks) that any changes (besides white space-only diff lines):

have 100% code coverage of the diff to origin/master
do not introduce a pylint warning type which is not disabled in pylintrc
do not introduce a new type of static analysis warning (as long as it is not currently suppressed) in mypy, pyre, and pytype.

Documentation on setting up local CI using tox and more is in README.md.

Follow-up:

The warnings shown (in the GitHub Actions Summary Page) are reminders that the PRs 22, 23, 24 are needed, continue with them.

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>

requirements-dev.txt

codecov · 2023-05-06T09:32:42Z

Codecov Report

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

@@            Coverage Diff            @@
##             master      #27   +/-   ##
=========================================
  Coverage          ?   69.00%           
=========================================
  Files             ?       20           
  Lines             ?     3420           
  Branches          ?        0           
=========================================
  Hits              ?     2360           
  Misses            ?     1060           
  Partials          ?        0

Flag	Coverage Δ
unittest	`69.00% <0.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Originally by Yann Dirson: With python3, pylint complains about `else: raise()` constructs. This rework avoids them and reduces cyclomatic complexity by using the error-out-first idiom. Minor related update by Bernhard Kaindl: .coveragerc: Don't complain if tests don't hit ValueError(pciids.txt) tests/test_pci.py covers this code except for the ValueError() lines. In .coveragerc, we already declare that coverage shall not complain about missing coverage on raising AssertionError and raise NotImplementedError, we do not expect to have to trigger "raise ValueError()" in general, so add ValueError() to the list. Signed-off-by: Yann Dirson <yann.dirson@vates.fr> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

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, Updated by Bernhard Kaindl to no longer need the WIP marker, to unblock the Python3 work, with more tests to be added. 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>

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

This is and update of the older commit "xcp.accessor: upgrade urllib usage (futurize)" by Yann Dirson, it reuses the changes from futurize and uses six.moves instead: To update xcp/accessor.py's use of urllib to Python3, Yann's Py3 PR originally used import future.standard_library.install_aliases(), which had several drawbacks: - Forced a new dependency on the future module - The future module is no longer updated and is using deprecated code - future.standard_library.install_aliases() imports imp - the 'import imp' itself triggers a deprecation warning - imp is slated for for removal with Python 3.12(might be later tough) - and it required him adding several pylint suppression comments - because future.standard_library also has no type markers, not using future but six.moves (which is also much shorter) also allows to use mypy without having the add additional warning suppressions. Further, using future would be inconsistent with Yann's existing Un-futurize in this commit: "Un-futurize: replace future.utils.raise_ with six.raise_from" So fully skip the future dependency and fix the deprecation warning by using six like Yann did in the commit above as well. Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

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

tox.ini

xcp/net/mac.py

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>

…n-not-iterating Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

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

Fixup the unclosed file warning which was introduced by commit 08f000 (xcp.repository: switch from ConfigParser to configparser) in xenserver#27. PS: Because I'm using pre-commit now, also remove a line which had trailing spaces Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

…le-warning xcp/repository.py: Fixup unclosed file warning left from #27

…/add-sar-system-load-testcase Add system load testcase to check collecting sar output, improve verification code

bernhardkaindl and others added 14 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>

bernhardkaindl force-pushed the py3-move-to-tox branch from be96bba to 48b21c3 Compare May 4, 2023 15:12

psafont reviewed May 4, 2023

View reviewed changes

requirements-dev.txt Outdated Show resolved Hide resolved

bernhardkaindl force-pushed the py3-move-to-tox branch from 48b21c3 to ca69f11 Compare May 6, 2023 09:31

bernhardkaindl and others added 6 commits May 8, 2023 15:13

xcp.xmlunwrap: Cover getIntAttribute() for fixing a pytype warning

b23911c

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

xcp/accessor.py: Fix test_accessor.py for Python3(more updates later)

99b845a

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

bernhardkaindl force-pushed the py3-move-to-tox branch from ca69f11 to b4e3aa0 Compare May 9, 2023 07:58

bernhardkaindl changed the title ~~Py3 move to tox~~ Initial Python3 support with Test-Driven-Development enforced by CI (with git diff coverage and diff lint checking) + mypy, pyre and pytype May 9, 2023

psafont reviewed May 9, 2023

View reviewed changes

tox.ini Outdated Show resolved Hide resolved

psafont approved these changes May 9, 2023

View reviewed changes

bernhardkaindl force-pushed the py3-move-to-tox branch 2 times, most recently from 6da737c to 421b4f7 Compare May 9, 2023 09:05

rosslagerwall reviewed May 9, 2023

View reviewed changes

xcp/net/mac.py Outdated Show resolved Hide resolved

bernhardkaindl mentioned this pull request May 9, 2023

Fix #18: encode() only if type is unicode (only for Py2) #23

Closed

bernhardkaindl added 12 commits May 9, 2023 15:12

Add generated Python typing stubs to allow for better mypy checks

c24c298

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>

Add stubs/branding.py for local checks(pytest,pylint,mypy,pytype,pyre)

8dfec59

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

pylint/mypy: Suppress three warnings (to not rewrite code)

bb53d84

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

Add run-pylint.py for checking and reporting pylint messages

8b3170c

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

xcp.net.mac: Suppress false positive from pylint --py3k: range-builti…

c9c5bc7

…n-not-iterating Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

xcp.version[pylint]: Add def __hash__(), py3-required-for def __eq__():

1a4f115

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

Add run-pyre.py: Run pyre static analysis(for integration to CI)

8879ac9

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

Add run-pytype.py: Run pytype (for static analysis in CI)

558a82d

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

mypy: Add pyproject.toml and supress mypy errors to be fixed later

1118553

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

Add tox.ini: Now, we have everything to run CI locally, using tox

6d653ec

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

With tox working, switch GitHub CI to run all checks using tox

814ca9d

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

Remove obsolete setup.py, requirements-dev.txt, and README

6c785e1

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

bernhardkaindl force-pushed the py3-move-to-tox branch from 421b4f7 to ec8e917 Compare May 9, 2023 13:39

Add README.md documenting the current project state

5110074

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

bernhardkaindl force-pushed the py3-move-to-tox branch from ec8e917 to 5110074 Compare May 9, 2023 13:51

rosslagerwall approved these changes May 9, 2023

View reviewed changes

bernhardkaindl merged commit 03b6a75 into xenserver:master May 9, 2023

This was referenced May 9, 2023

Testsuite driven py3 #17

Closed

Test driven py3, for merge, with full CI #26

Closed

bernhardkaindl deleted the py3-move-to-tox branch May 17, 2023 22:10

bernhardkaindl mentioned this pull request Jun 7, 2023

xcp/repository.py: Fixup unclosed file warning left from #27 #73

Merged

bernhardkaindl added a commit that referenced this pull request Jun 9, 2023

Merge pull request #73 from xenserver-next/repository-fix-unclosed-fi…

5d7c44e

…le-warning xcp/repository.py: Fixup unclosed file warning left from #27

This was referenced Jun 9, 2023

Add Accessor.openText() as context manager for openAddress() #76

Merged

TDD Py3 merge: Commits up to test_accessor.py: Test reading binary data from http and file #25

Closed

bernhardkaindl mentioned this pull request Mar 6, 2024

Changing xcp.xmlunwrap API to use unicode (py3:str) instead of str (py3:bytes) #18

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Initial Python3 support with Test-Driven-Development enforced by CI (with git diff coverage and diff lint checking) + mypy, pyre and pytype #27

Initial Python3 support with Test-Driven-Development enforced by CI (with git diff coverage and diff lint checking) + mypy, pyre and pytype #27

Uh oh!

bernhardkaindl commented May 4, 2023 •

edited

Loading

Uh oh!

Uh oh!

codecov bot commented May 6, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Initial Python3 support with Test-Driven-Development enforced by CI (with git diff coverage and diff lint checking) + mypy, pyre and pytype #27

Initial Python3 support with Test-Driven-Development enforced by CI (with git diff coverage and diff lint checking) + mypy, pyre and pytype #27

Uh oh!

Conversation

bernhardkaindl commented May 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Initial Python3 support with Test-Driven-Development enforced by CI:

Uh oh!

Uh oh!

codecov bot commented May 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bernhardkaindl commented May 4, 2023 •

edited

Loading

codecov bot commented May 6, 2023 •

edited

Loading