swansonk14 / typed-argument-parser

Typed argument parser for Python
MIT License
505 stars 40 forks source link

Union code coverage #132

Closed kddubey closed 6 months ago

kddubey commented 6 months ago

128 reduced code coverage b/c the current coverage workflow runs the tests w/o pydantic installed.

We want the union of lines covered by the 3 testing environments in the testing workflow: one without pydantic installed, one with pydantic<2, and one with pydantic>=2. This funcionality is achieved through pytest-cov's --cov-append feature.

How has this been tested?

Locally. Create 3 virtual environments: one without pydantic installed, one with pydantic<2, and one with pydantic>=2. Then simulate the test workflow:

workon tap-wo-pydantic && \
pytest --cov=tap && \
workon tap-pydantic-v1 && \
pytest --cov=tap --cov-append && \
workon tap && \
pytest --cov=tap --cov-append

This command outputs 3 coverage reports (one for each pytest invocation):

# VIRTUAL ENVIRONMENT: tap-wo-pydantic
# test output omitted

---------- coverage: platform darwin, python 3.11.8-final-0 ----------
Name              Stmts   Miss  Cover
-------------------------------------
tap/__init__.py       5      0   100%
tap/_version.py       3      0   100%
tap/tap.py          290     21    93%
tap/tapify.py       156     22    86%
tap/utils.py        230     22    90%
-------------------------------------
TOTAL               684     65    90%

# VIRTUAL ENVIRONMENT: tap-pydantic-v1
# test output omitted

---------- coverage: platform darwin, python 3.11.8-final-0 ----------
Name              Stmts   Miss  Cover
-------------------------------------
tap/__init__.py       5      0   100%
tap/_version.py       3      0   100%
tap/tap.py          290     21    93%
tap/tapify.py       156     14    91%
tap/utils.py        230     22    90%
-------------------------------------
TOTAL               684     57    92%

# VIRTUAL ENVIRONMENT: tap
# test output omitted

---------- coverage: platform darwin, python 3.11.8-final-0 ----------
Name              Stmts   Miss  Cover
-------------------------------------
tap/__init__.py       5      0   100%
tap/_version.py       3      0   100%
tap/tap.py          290     21    93%
tap/tapify.py       156      3    98%
tap/utils.py        230     22    90%
-------------------------------------
TOTAL               684     46    93%

The final .coverage file that should be sent to codecov.io is that last one (obtained locally via coverage report -m):

Name              Stmts   Miss  Cover   Missing
-----------------------------------------------
tap/__init__.py       5      0   100%
tap/_version.py       3      0   100%
tap/tap.py          290     21    93%   210, 239-241, 402-404, 438, 467, 514, 558, 561-566, 587, 686-687, 717, 725
tap/tapify.py       156      3    98%   89, 143, 165
tap/utils.py        230     22    90%   83-85, 147, 279, 305, 426, 439-455, 487-491
-----------------------------------------------
TOTAL               684     46    93%

So this PR is correct if and only if the coverage seen in codecov.io matches the output immediately above. Expect coverage to be: (684-46)/684 = 93.27%.

Explanation of code coverage

Recall that #128 only touches tap/__init__.py and tap/tapify.py. 2 of the 3 uncovered lines in tap/tapify.py are inconsequential:

165 raises a TypeError that can be hit if a user inputs a data model containing a field that isn't a Pydantic model field, Pydantic dataclass field, or builtin dataclass field. This should be tested later, but hopefully it's ok for now :-)

codecov-commenter commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.00%. Comparing base (8147f75) to head (b975ebf).

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #132 +/- ## ========================================== + Coverage 90.49% 94.00% +3.50% ========================================== Files 5 5 Lines 684 684 ========================================== + Hits 619 643 +24 + Misses 65 41 -24 ``` | [Flag](https://app.codecov.io/gh/swansonk14/typed-argument-parser/pull/132/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/swansonk14/typed-argument-parser/pull/132/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kddubey commented 6 months ago

So this PR is correct if and only if the coverage seen in codecov.io matches the output immediately above. Expect coverage to be: (684-46)/684 = 93.27%.

Coverage turned out to be 94% but I'm not totally sure why lol

Edit: I think it's b/c there are Python version-dependent clauses that my local test didn't hit

martinjm97 commented 6 months ago

@kddubey , this looks excellent to me! Thank you for adding the missing link too. As long as @swansonk14 approves, it should be ready for release.

--Jesse

swansonk14 commented 6 months ago

This looks awesome, thank you @kddubey!