From 1b0bbb9a560fea11f71f567384ffd52375d4ef7c Mon Sep 17 00:00:00 2001 From: Athanasius Date: Fri, 30 Sep 2022 19:25:22 +0100 Subject: [PATCH 1/7] tests: Improved coverage reporting * Always report on coverage, if no tests failed. * Remove `.coveragerc`, in favour of `pyproject.toml`. * Use `coverage-conditional-plugin`: - Two rules added, `sys-platform-win32` and `sys-platform-not-win32`. - Those rules used so non-win32 code run on win32 doesn't cause coverage to be reported as less than 100%. There's the assumption that !win32 means Linux, probably. --- .coveragerc | 6 ------ journal_lock.py | 8 ++++---- pyproject.toml | 9 ++++++++- requirements-dev.txt | 1 + 4 files changed, 13 insertions(+), 11 deletions(-) delete mode 100644 .coveragerc diff --git a/.coveragerc b/.coveragerc deleted file mode 100644 index 019facb4..00000000 --- a/.coveragerc +++ /dev/null @@ -1,6 +0,0 @@ -[run] -omit = - # The tests themselves - tests/* - # Any venv files - venv/* diff --git a/journal_lock.py b/journal_lock.py index 91a7895f..b96fd9e9 100644 --- a/journal_lock.py +++ b/journal_lock.py @@ -94,7 +94,7 @@ class JournalLock: :return: LockResult - See the class Enum definition """ - if sys.platform == 'win32': + if sys.platform == 'win32': # pragma: sys-platform-not-win32 logger.trace_if('journal-lock', 'win32, using msvcrt') # win32 doesn't have fcntl, so we have to use msvcrt import msvcrt @@ -107,7 +107,7 @@ class JournalLock: f", assuming another process running: {e!r}") return JournalLockResult.ALREADY_LOCKED - else: # pytest coverage only sees this on !win32 + else: # pragma: sys-platform-win32 logger.trace_if('journal-lock', 'NOT win32, using fcntl') try: import fcntl @@ -143,7 +143,7 @@ class JournalLock: return True # We weren't locked, and still aren't unlocked = False - if sys.platform == 'win32': + if sys.platform == 'win32': # pragma: sys-platform-not-win32 logger.trace_if('journal-lock', 'win32, using msvcrt') # win32 doesn't have fcntl, so we have to use msvcrt import msvcrt @@ -160,7 +160,7 @@ class JournalLock: else: unlocked = True - else: # pytest coverage only sees this on !win32 + else: # pragma: sys-platform-win32 logger.trace_if('journal-lock', 'NOT win32, using fcntl') try: import fcntl diff --git a/pyproject.toml b/pyproject.toml index a7f41ad4..387f00c7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,9 +7,16 @@ line_length = 119 [tool.pytest.ini_options] testpaths = ["tests"] # Search for tests in tests/ +addopts = "--cov . --cov plugins --cov-report=term-missing --no-cov-on-fail" +# --cov-fail-under 80" [tool.coverage.run] -omit = ["venv/*"] # when running pytest --cov, dont report coverage in venv directories +omit = [ "tests/*", "venv/*", "dist.win32/*" ] +plugins = [ "coverage_conditional_plugin" ] + +[tool.coverage.coverage_conditional_plugin.rules] +sys-platform-win32 = "sys_platform == 'win32'" +sys-platform-not-win32 = "sys_platform != 'win32'" [tool.pyright] # pythonPlatform = 'Darwin' diff --git a/requirements-dev.txt b/requirements-dev.txt index a9f569f5..2ae38e0a 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -43,6 +43,7 @@ py2exe==0.12.0.1; sys_platform == 'win32' pytest==7.1.3 pytest-cov==4.0.0 # Pytest code coverage support coverage[toml]==6.5.0 # pytest-cov dep. This is here to ensure that it includes TOML support for pyproject.toml configs +coverage-conditional-plugin==0.7.0 # For manipulating folder permissions and the like. pywin32==304; sys_platform == 'win32' From 5efd27a83cbabdc716f096c1cc6fd069460f2f55 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Fri, 30 Sep 2022 19:45:12 +0100 Subject: [PATCH 2/7] tests: Attempt to fix config/ coverage * Define `darwin` and `linux` *and* "platform known" pragmas. * Use per-platform pragmas in `config/__init__.py` selection of implementation. * Attempt, and fail, to use pragma in `config/darwin.py` to ignore it on other platforms. --- config/__init__.py | 8 ++++---- config/darwin.py | 4 ++++ pyproject.toml | 5 +++++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/config/__init__.py b/config/__init__.py index cc125129..6f5bfa12 100644 --- a/config/__init__.py +++ b/config/__init__.py @@ -454,19 +454,19 @@ def get_config(*args, **kwargs) -> AbstractConfig: :param kwargs: Args to be passed through to implementation. :return: Instance of the implementation. """ - if sys.platform == "darwin": + if sys.platform == "darwin": # pragma: sys-platform-not-darwin from .darwin import MacConfig return MacConfig(*args, **kwargs) - elif sys.platform == "win32": + elif sys.platform == "win32": # pragma: sys-platform-not-win32 from .windows import WinConfig return WinConfig(*args, **kwargs) - elif sys.platform == "linux": + elif sys.platform == "linux": # pragma: sys-platform-not-linux from .linux import LinuxConfig return LinuxConfig(*args, **kwargs) - else: + else: # pragma: sys-platform-known raise ValueError(f'Unknown platform: {sys.platform=}') diff --git a/config/darwin.py b/config/darwin.py index eb2b887f..a492b8a1 100644 --- a/config/darwin.py +++ b/config/darwin.py @@ -1,3 +1,7 @@ +"""Darwin/macOS implementation of AbstractConfig.""" +# This doesn't actually work: +# +# pragma: sys-platform-not-darwin import pathlib import sys from typing import Any, Dict, List, Union diff --git a/pyproject.toml b/pyproject.toml index 387f00c7..bf2fcc52 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,6 +17,11 @@ plugins = [ "coverage_conditional_plugin" ] [tool.coverage.coverage_conditional_plugin.rules] sys-platform-win32 = "sys_platform == 'win32'" sys-platform-not-win32 = "sys_platform != 'win32'" +sys-platform-darwin = "sys_platform == 'darwin'" +sys-platform-not-darwin = "sys_platform != 'darwin'" +sys-platform-linux = "sys_platform == 'linux'" +sys-platform-not-linux = "sys_platform != 'linux'" +sys-platform-known = "sys_platform in ('darwin', 'linux', 'win32')" [tool.pyright] # pythonPlatform = 'Darwin' From baf62f03fd5cef12fa7bb9fbc2239e7f93450042 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Sun, 2 Oct 2022 12:11:16 +0100 Subject: [PATCH 3/7] pytest/coverage: Resolve the "which way around to have pragmas" issue 1. You end up either inverting the sense of a `coverage_conditional_plugin` pragma's name (versus what it actually tests), *or* where you put it in the code. 2. As the pragmas are only defined in once, in one place, it's better to invert the sense there, rather than in *every single use case*. Then technically any 'other' branch isn't guaranteed to --- config/__init__.py | 8 ++++---- journal_lock.py | 8 ++++---- pyproject.toml | 16 +++++++++------- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/config/__init__.py b/config/__init__.py index 6f5bfa12..56679019 100644 --- a/config/__init__.py +++ b/config/__init__.py @@ -454,19 +454,19 @@ def get_config(*args, **kwargs) -> AbstractConfig: :param kwargs: Args to be passed through to implementation. :return: Instance of the implementation. """ - if sys.platform == "darwin": # pragma: sys-platform-not-darwin + if sys.platform == "darwin": # pragma: sys-platform-darwin from .darwin import MacConfig return MacConfig(*args, **kwargs) - elif sys.platform == "win32": # pragma: sys-platform-not-win32 + elif sys.platform == "win32": # pragma: sys-platform-win32 from .windows import WinConfig return WinConfig(*args, **kwargs) - elif sys.platform == "linux": # pragma: sys-platform-not-linux + elif sys.platform == "linux": # pragma: sys-platform-linux from .linux import LinuxConfig return LinuxConfig(*args, **kwargs) - else: # pragma: sys-platform-known + else: # pragma: sys-platform-not-known raise ValueError(f'Unknown platform: {sys.platform=}') diff --git a/journal_lock.py b/journal_lock.py index b96fd9e9..ef5cf983 100644 --- a/journal_lock.py +++ b/journal_lock.py @@ -94,7 +94,7 @@ class JournalLock: :return: LockResult - See the class Enum definition """ - if sys.platform == 'win32': # pragma: sys-platform-not-win32 + if sys.platform == 'win32': # pragma: sys-platform-win32 logger.trace_if('journal-lock', 'win32, using msvcrt') # win32 doesn't have fcntl, so we have to use msvcrt import msvcrt @@ -107,7 +107,7 @@ class JournalLock: f", assuming another process running: {e!r}") return JournalLockResult.ALREADY_LOCKED - else: # pragma: sys-platform-win32 + else: # pragma: sys-platform-not-win32 logger.trace_if('journal-lock', 'NOT win32, using fcntl') try: import fcntl @@ -143,7 +143,7 @@ class JournalLock: return True # We weren't locked, and still aren't unlocked = False - if sys.platform == 'win32': # pragma: sys-platform-not-win32 + if sys.platform == 'win32': # pragma: sys-platform-win32 logger.trace_if('journal-lock', 'win32, using msvcrt') # win32 doesn't have fcntl, so we have to use msvcrt import msvcrt @@ -160,7 +160,7 @@ class JournalLock: else: unlocked = True - else: # pragma: sys-platform-win32 + else: # pragma: sys-platform-not-win32 logger.trace_if('journal-lock', 'NOT win32, using fcntl') try: import fcntl diff --git a/pyproject.toml b/pyproject.toml index bf2fcc52..7086d166 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,13 +15,15 @@ omit = [ "tests/*", "venv/*", "dist.win32/*" ] plugins = [ "coverage_conditional_plugin" ] [tool.coverage.coverage_conditional_plugin.rules] -sys-platform-win32 = "sys_platform == 'win32'" -sys-platform-not-win32 = "sys_platform != 'win32'" -sys-platform-darwin = "sys_platform == 'darwin'" -sys-platform-not-darwin = "sys_platform != 'darwin'" -sys-platform-linux = "sys_platform == 'linux'" -sys-platform-not-linux = "sys_platform != 'linux'" -sys-platform-known = "sys_platform in ('darwin', 'linux', 'win32')" +# Yes, the sense of all of these is inverted, because else it ends up +# inverted at *every* use. +sys-platform-win32 = "sys_platform != 'win32'" +sys-platform-not-win32 = "sys_platform == 'win32'" +sys-platform-darwin = "sys_platform != 'darwin'" +sys-platform-not-darwin = "sys_platform == 'darwin'" +sys-platform-linux = "sys_platform != 'linux'" +sys-platform-not-linux = "sys_platform == 'linux'" +sys-platform-not-known = "sys_platform in ('darwin', 'linux', 'win32')" [tool.pyright] # pythonPlatform = 'Darwin' From 7aa832e3d10122f788394e4792453094ce4814d5 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Sun, 2 Oct 2022 12:31:11 +0100 Subject: [PATCH 4/7] Contributing.md: test coverage notes --- Contributing.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/Contributing.md b/Contributing.md index 4d7fc9fc..6d60065a 100644 --- a/Contributing.md +++ b/Contributing.md @@ -244,6 +244,42 @@ handy if you want to step through the testing code to be sure of anything. Otherwise, see the [pytest documentation](https://docs.pytest.org/en/stable/contents.html). +### Test Coverage +As we work towards actually having tests for as much of the code as possible +it is useful to monitor the current test coverage. + +Running `pytest` will also produce the overall coverage report, see the +configured options in `pyproject.toml`. + +One issue you might run into is where there is code that only runs on one +platform. By default `pytest-cov`/`coverage` will count this code as not +tested when run on a different platform. We utilise the +`coverage-conditional-plugin` module so that `#pragma` comments can be used +to give hints to coverage about this. + +The pragmas are defined in the +`tool.coverage.coverage_conditional_plugin.rules` section of `pyproject.toml`, +e.g. + +```toml +[tool.coverage.coverage_conditional_plugin.rules] +# Yes, the sense of all of these is inverted, because else it ends up +# inverted at *every* use. +sys-platform-win32 = "sys_platform != 'win32'" +... +``` +And are used as in: +```python +import sys + +if sys.platform == 'win32': # pragma: sys-platform-win32 + ... +else: # pragma: sys-platform-not-win32 + ... +``` +Note the inverted sense of the pragma definitions, as the comments cause +`coverage` to *not* consider that code block on this platform. + --- ## Imports used only in core plugins From 80a8a32666808dc875b0d8bc00a2c0670a52c4cb Mon Sep 17 00:00:00 2001 From: Athanasius Date: Sun, 2 Oct 2022 12:38:24 +0100 Subject: [PATCH 5/7] pytest/coverage: Improve the .toml comment about pragma 'inversion' --- Contributing.md | 8 ++++++-- pyproject.toml | 6 ++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Contributing.md b/Contributing.md index 6d60065a..29135804 100644 --- a/Contributing.md +++ b/Contributing.md @@ -263,8 +263,6 @@ e.g. ```toml [tool.coverage.coverage_conditional_plugin.rules] -# Yes, the sense of all of these is inverted, because else it ends up -# inverted at *every* use. sys-platform-win32 = "sys_platform != 'win32'" ... ``` @@ -280,6 +278,12 @@ else: # pragma: sys-platform-not-win32 Note the inverted sense of the pragma definitions, as the comments cause `coverage` to *not* consider that code block on this platform. +As of 2022-10-02 and `coverage-conditional-plugin==0.7.0` there is no way to +signal that an entire file should be excluded from coverage reporting on the +current platform. See +[this GitHub issue comment](https://github.com/wemake-services/coverage-conditional-plugin/issues/2#issuecomment-1263918296) +. + --- ## Imports used only in core plugins diff --git a/pyproject.toml b/pyproject.toml index 7086d166..bc3a32b0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,8 +15,10 @@ omit = [ "tests/*", "venv/*", "dist.win32/*" ] plugins = [ "coverage_conditional_plugin" ] [tool.coverage.coverage_conditional_plugin.rules] -# Yes, the sense of all of these is inverted, because else it ends up -# inverted at *every* use. +# NB: The name versus content of all of these are inverted because of the way +# they're used. When a pragma cites one it causes that code block to +# **NOT** be considered for code coverage. +# See Contributin.md#test-coverage for more details. sys-platform-win32 = "sys_platform != 'win32'" sys-platform-not-win32 = "sys_platform == 'win32'" sys-platform-darwin = "sys_platform != 'darwin'" From bf7be4dc5ae55d7351bfe0c70569ba83d31f4122 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Sun, 2 Oct 2022 12:39:08 +0100 Subject: [PATCH 6/7] config/darwin: Remove non-functional coverage pragma --- config/darwin.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/config/darwin.py b/config/darwin.py index a492b8a1..895218a8 100644 --- a/config/darwin.py +++ b/config/darwin.py @@ -1,7 +1,4 @@ """Darwin/macOS implementation of AbstractConfig.""" -# This doesn't actually work: -# -# pragma: sys-platform-not-darwin import pathlib import sys from typing import Any, Dict, List, Union From eceaa4f0be4fb89229430a5eb32ec42205886df0 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Sun, 2 Oct 2022 19:46:27 +0100 Subject: [PATCH 7/7] pyproject.toml: Fix typoed 'Contributing.md' --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index bc3a32b0..91c6e7e3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,7 @@ plugins = [ "coverage_conditional_plugin" ] # NB: The name versus content of all of these are inverted because of the way # they're used. When a pragma cites one it causes that code block to # **NOT** be considered for code coverage. -# See Contributin.md#test-coverage for more details. +# See Contributing.md#test-coverage for more details. sys-platform-win32 = "sys_platform != 'win32'" sys-platform-not-win32 = "sys_platform == 'win32'" sys-platform-darwin = "sys_platform != 'darwin'"