Skip to content

Adding asyncio unit tests#22

Open
bradpenney wants to merge 2 commits into
firestoned:mainfrom
bradpenney:adding-asyncio-unit-tests
Open

Adding asyncio unit tests#22
bradpenney wants to merge 2 commits into
firestoned:mainfrom
bradpenney:adding-asyncio-unit-tests

Conversation

@bradpenney
Copy link
Copy Markdown
Contributor

Summary

  • Expand CLI test coverage to validate logging setup plus every custom click.ParamType, including new negative cases and alias checks.
  • Add resource loader tests for JSON/YAML, $ref resolution, and validation errors, alongside _jsonloader URI normalization.
  • Bring new tests into pylint compliance and document the required lint/coverage workflow for contributors.
  • Update contributor guidance to mandate Python 3.12+, local lint + coverage runs, and explain how (and when) to call module-private helpers.

Signed-off-by: Brad Penney <penney.brad@protonmail.com>
@bradpenney
Copy link
Copy Markdown
Contributor Author

pytest --cov=firestone_lib test/ --cov-report term-missing   
======================================= test session starts ========================================
platform linux -- Python 3.14.0, pytest-8.3.5, pluggy-1.5.0
rootdir: /home/brad/Documents/firestoned/firestone-lib
configfile: pyproject.toml
plugins: mock-3.14.0, cov-7.0.0
collected 36 items                                                                                 

test/test_cli.py ..........................                                                  [ 72%]
test/test_resource.py .....                                                                  [ 86%]
test/test_utils.py .....                                                                     [100%]

========================================== tests coverage ==========================================
_________________________ coverage: platform linux, python 3.14.0-final-0 __________________________

Name                                          Stmts   Miss  Cover   Missing
---------------------------------------------------------------------------
firestone_lib/__init__.py                         0      0   100%
firestone_lib/cli.py                            112      0   100%
firestone_lib/resource.py                        34      0   100%
firestone_lib/resources/__init__.py               0      0   100%
firestone_lib/resources/logging/__init__.py       0      0   100%
firestone_lib/utils.py                           16      0   100%
---------------------------------------------------------------------------
TOTAL                                           162      0   100%
======================================== 36 passed in 0.12s ========================================

  ~/Doc/f/firestone-lib on   adding-asyncio-unit-tests      firestone-lib-py3.14 at  14:56:27
❯ pylint .                                                     

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

@bradpenney bradpenney force-pushed the adding-asyncio-unit-tests branch 4 times, most recently from 33aa333 to 199dfe5 Compare October 26, 2025 18:09
@bradpenney bradpenney force-pushed the adding-asyncio-unit-tests branch 2 times, most recently from 011deb5 to 4aa8f81 Compare October 26, 2025 18:48
Comment thread .github/workflows/pr.yml
@bradpenney bradpenney force-pushed the adding-asyncio-unit-tests branch 2 times, most recently from 64f3b5b to d54b76a Compare October 26, 2025 19:23
Comment thread firestone_lib/resource.py
def _jsonloader(uri, **kwargs):
if uri.startswith("file://"):
uri = uri[8:]
uri = uri[7:]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • _jsonloader now strips file:// URIs with uri[7:] and adds a fallback for plain file: . The old slice uri[8:] lopped off the first character of absolute paths like file:///tmp/schema.yaml, so jsonref tried to open tmp/schema.yaml relative to CWD and crashed whenever a schema contained $refs to other local files or when tests exercised those paths.
  • Guarding the plain file: case lets jsonref hand us either file:///... or file:/... and still resolves correctly, which is how coverage tests trigger the loader.
  • With this correction, nested schema references load reliably whether they arrive with file: or file:// URIs, eliminating the FileNotFound errors seen in test runs.

@ebourgeois
Copy link
Copy Markdown
Contributor

Please update the builds to match these new python version constraints

@bradpenney bradpenney force-pushed the adding-asyncio-unit-tests branch 4 times, most recently from 5fa1069 to 89a4adf Compare October 26, 2025 19:48
Signed-off-by: Brad Penney <penney.brad@protonmail.com>
@bradpenney bradpenney force-pushed the adding-asyncio-unit-tests branch from 89a4adf to 1273178 Compare October 26, 2025 22:28
@bradpenney bradpenney requested a review from ebourgeois October 26, 2025 22:29
@bradpenney
Copy link
Copy Markdown
Contributor Author

Please update the builds to match these new python version constraints

Done! https://github.com/firestoned/firestone-lib/actions/runs/18824634843

Copy link
Copy Markdown
Contributor

@ebourgeois ebourgeois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gtg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants