Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code coverage is not 100% #8

Open
winksaville opened this issue Oct 8, 2020 · 4 comments
Open

Code coverage is not 100% #8

winksaville opened this issue Oct 8, 2020 · 4 comments

Comments

@winksaville
Copy link
Owner

  • taperable_helix version: 0.8.9
  • Python version: 3.7
  • Operating System: Linux

Code coverage is not 100%

The reason is that there is an extraenous "if" statement that
can't be tested. This is necessary so make mypy works and
if mypy worked the "if" statement wouldn't be necessary and
we'd then have 100% coverage.

What I Did

The reason make coverage is not 100%:

(py-helix) wink@3900x:~/prgs/CadQuery/projects/taperable-helix (master)
$ make coverage
coverage run --source taperable_helix -m pytest
====================================================== test session starts =======================================================
platform linux -- Python 3.7.8, pytest-6.1.1, py-1.9.0, pluggy-0.13.1
rootdir: /home/wink/prgs/CadQuery/projects/taperable-helix
collected 19 items                                                                                                               

tests/test_taperable_helix.py ...................                                                                          [100%]

======================================================= 19 passed in 0.37s =======================================================
coverage report -m
Name                          Stmts   Miss  Cover   Missing
-----------------------------------------------------------
taperable_helix/__init__.py       4      0   100%
taperable_helix/helix.py         53      1    98%   176
-----------------------------------------------------------
TOTAL                            57      1    98%
# coverage html
# python -c "$BROWSER_PYSCRIPT" htmlcov/index.html

Because to allow make mypy to pass:

(py-helix) wink@3900x:~/prgs/CadQuery/projects/taperable-helix (master)
$ make mypy 
mypy setup.py taperable_helix/ tests/ examples/ docs/
Success: no issues found in 8 source files

The following code is added to helix.py:

            # This if statement is needed to satisfy mypy this is already
            # guaranteed in helix() above so the if statement is always
            # False. Furthermore this means the raise line is untestable.
            # Hopefully this can be fixed in the future.
            if (hl is None) or (hl.radius is None):
                raise ValueError("hl or hl.radius is None, should never happen")

If you comment the code the following are the results of make mypy:

(py-helix) wink@3900x:~/prgs/CadQuery/projects/taperable-helix (master)
$ make mypy 
mypy setup.py taperable_helix/ tests/ examples/ docs/
taperable_helix/helix.py:206: error: Item "None" of "Optional[HelixLocation]" has no attribute "radius"
taperable_helix/helix.py:206: error: Unsupported operand types for + ("None" and "float")
taperable_helix/helix.py:206: note: Both left and right operands are unions
taperable_helix/helix.py:206: error: Item "None" of "Optional[HelixLocation]" has no attribute "horz_offset"
taperable_helix/helix.py:213: error: Item "None" of "Optional[HelixLocation]" has no attribute "vert_offset"
Found 4 errors in 1 file (checked 8 source files)
make: *** [Makefile:93: mypy] Error 1
@winksaville
Copy link
Owner Author

I tested this with python 3.7 and 3.8, the same results.
I'm hoping that soon I can test with python 3.9 and maybe
things will work. Right now trying to install 3.9 in conda,
conda create -n py39-helix python=3.9 fails because conda-forge
hasn't been able to compile all dependencies I need for developement
using 3.9, hopefully that will be resolved soon.

@winksaville
Copy link
Owner Author

TODO: File a bug on mypy?

@winksaville
Copy link
Owner Author

A solution would be don't be "tricky" with using Optionals, but I think its "useful". We'll see :)

@winksaville
Copy link
Owner Author

Another solution would be to add a test field in Helix so that path could be tested, but I don't like that ether, LoL.

winksaville added a commit that referenced this issue Oct 8, 2020
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

No branches or pull requests

1 participant