manrajgrover / halo

💫 Beautiful spinners for terminal, IPython and Jupyter
MIT License
2.89k stars 147 forks source link

Tests: Add tests that will generate more coverage #94

Closed Lilja closed 6 years ago

Lilja commented 6 years ago

Description of new feature, or changes

Adds more tests and coverage

Checklist

Related Issues and Discussions

Fixes #88

People to notify

@manrajgrover

Lilja commented 6 years ago
.................................................
Name                    Stmts   Miss Branch BrPart  Cover   Missing
-------------------------------------------------------------------
halo/__init__.py            6      0      0      0   100%
halo/_utils.py             46      9     10      3    79%   9-10, 44-45, 51, 53, 57-58, 130, 50->51, 52->53, 129->130
halo/halo.py              181      6     46      4    96%   79, 82-85, 394, 423, 81->82, 393->394, 414->418, 422->423
halo/halo_notebook.py      57      2     12      2    94%   55, 84, 54->55, 83->84
-------------------------------------------------------------------
TOTAL                     290     17     68      9    93%
----------------------------------------------------------------------
manrajgrover commented 6 years ago

@Lilja Looks like we are not testing utilities well. We can improve on that if you would like to take it up in this PR

Lilja commented 6 years ago

@manrajgrover Thanks for your feedback. Do you want me to create multiple commits or stay strict to just one?

I had no idea that Appveyor was for windows. I'm reverting the OS specific stuff as well as the mock stuff.

My analysis of _utils.py is that 90% of the untested stuff is related to ipython or jython. I've have no clue what they are.

Also, some of the coverage will not be reported on windows whereas on linux it will be reported. So the coverage report needs to be a merge of these two. You might already know this but I'd figure that I'd just tell you anyway!

manrajgrover commented 6 years ago

@Lilja

Thanks for your feedback. Do you want me to create multiple commits or stay strict to just one?

Please add more commits to this PR only. Don't worry about that.

My analysis of _utils.py is that 90% of the untested stuff is related to ipython or jython. I've have no clue what they are.

It would be ipython. Testing it would be similar to testing it in terminal except for text width (if I remember correctly)

Also, some of the coverage will not be reported on windows whereas on linux it will be reported. So the coverage report needs to be a merge of these two. You might already know this but I'd figure that I'd just tell you anyway!

Yup, I'm aware of that :) Thanks for remind though

Lilja commented 6 years ago

@manrajgrover Allright. Maybe Ipython tests could be another PR?

manrajgrover commented 6 years ago

@Lilja Yup, you can raise a separate PR for it