aclark4life / vanity

Get package download statistics from PyPI
GNU General Public License v2.0
66 stars 14 forks source link

Add tests #33

Closed mattjegan closed 4 years ago

mattjegan commented 7 years ago

There are not unit tests available as of yet in this repo apart from the general "does it run" TravisCI tests. Perhaps this could be opened up and labelled as "hacktoberfest" to draw attention @aclark4life. It may help bring more contributors to the project.

hugovk commented 7 years ago

Labelled!

pratyushprakash commented 7 years ago

Hi, I'd like to take this up. Can you elaborate on what tests should be included

M3rs commented 7 years ago

I'm sorry, should I have commented here first? I added some tests for the "normalize" method and created a pull request.

hugovk commented 7 years ago

Thanks @M3rs for the unit tests for normalize added in PR https://github.com/aclark4life/vanity/pull/35!

And there's still other functions to unit test :)

killvung commented 7 years ago

Hey! I just looked at the normalize() function, but wondering whether we have to take action or not, if the user passes in string with space. Because it's technically possible to do it by calling

vanity " Flask "

So would vanity suppose to evaluation as correct Flask package or error?

mattjegan commented 7 years ago

@killVung I would argue that although it's possible to input something like vanity " Flask " we should raise an error since PEP8 does not allow whitespace in package names.

At the moment vanity seems to accept the input but doesn't interpret it as expected, I have opened an issue for this in #42

hugovk commented 7 years ago

I think the most useful thing for the user would be to trim the leading and trailing whitespace.

hugovk commented 7 years ago

Without the "does it run" tests and only unit tests, we'd be at 31% coverage.

You can clearly see here which functions aren't yet covered and would benefit from unit tests:

https://coveralls.io/builds/8285536/source?filename=vanity.py

M3rs commented 7 years ago

Added some more tests (testing the by_two function), PR #50 -- not sure why adding tests dropped coverage by 0.2%.

hugovk commented 7 years ago

Coverage dropped because by_two was refactored into fewer lines:

No problem.

hugovk commented 7 years ago

PR #50 brings unit-test-only coverage up to 36%.

https://coveralls.io/builds/8321165/source?filename=vanity.py

Forward‚ onward‚ and upward!

M3rs commented 7 years ago

Working on tests for count_downloads method next, I'll make another PR :)

M3rs commented 7 years ago

PR #55 opened, basic tests for "get_jsonparsed_data".

hugovk commented 7 years ago

PR https://github.com/aclark4life/vanity/pull/54 brings coverage to 46%!

hugovk commented 7 years ago

PR #55 to 51%!

aclark4life commented 7 years ago

GO GO GO

M3rs commented 7 years ago

Could you post the link to the unit-test-only coverage again? Working on more =)

hugovk commented 7 years ago

I'm not at a computer to push that branch right now, but you can check yourself with something like:

coverage run --include=vanity.py tests.py # runs only unit tests with coverage
coverage report # shows percentage on CL
coverage html # open htmlcov/index.html for details
M3rs commented 7 years ago

Thanks!

M3rs commented 7 years ago

PR #56 created, takes unit-test-only coverage on vanity.py up to 68%!

Biggest chunk of code missing tests is get_releases and the section of get_release_info dependent on get_releases.

hugovk commented 7 years ago

PR https://github.com/aclark4life/vanity/pull/56 to 69%!

suv27 commented 4 years ago

Is this issue still open, I can to practice my unit test skills, I would like to know if there is any python script that still needs to be unit tests?

hugovk commented 4 years ago

Unfortunately the API that this project used is no longer available, so it's probably time to say thank you and archive this repo.

Alternatives:

suv27 commented 4 years ago

@hugovk Those alternative repos have issues about unit testing that I can work on? What about https://github.com/hugovk/pypistats?

hugovk commented 4 years ago

https://github.com/hugovk/pypistats is 99.55% covered by unit tests, which is good enough. Thanks for asking!

guntbert commented 4 years ago

@hugovk might be a good idea to close this issue and remove the hacktoberfest label?

hugovk commented 4 years ago

Good idea, though I don't have permission to do so.

@aclark4life Please could you unlabel and close this?