ljcooke / see

Python's dir() for humans.
https://ljcooke.github.io/see/
BSD 3-Clause "New" or "Revised" License
245 stars 18 forks source link

Add unit tests #8

Closed hugovk closed 8 years ago

hugovk commented 8 years ago

TODO Before merge, please will you enable the araile/see repo at:

ljcooke commented 8 years ago

Thanks for this. I wasn't sure how to go about unit-testing for this project but this looks like a great start.

I hope to find some time later today to investigate it properly. My initial thoughts:

hugovk commented 8 years ago

At a glance, coverage/Coveralls seems to ignore the ~100 lines of code that define SYMBOLS. Is that right? (I'm not experienced with measuring code coverage, so I'm not sure how this works.)

Looks like coverage counts SYMBOLS as a single, covered line. Line 222 is green (covered), the rest are ignored: https://coveralls.io/jobs/13671203/source_files/260672864#L222

(The local output of coverage html agrees.)

There's a lot of stuff added to the .gitignore file. Is much of this needed for testing? I'd rather keep .gitignore to the minimum required for the project.

The .gitignore has the common Python things from https://github.com/github/gitignore/blob/master/README.md added, but the least needed for coverage is only to ignore .coverage and htmlcov/. Would you like me trim that back down and only add those needed ones?

If it all works, I'd like to write more unit tests. Would it be possible to put test_see.py in a tests directory and run that directory as a test suite? (Edited to add: would this approach also work with coverage?)

Yes, that's possible, and it also works with coverage. Would you like me to refactor it like that?

ljcooke commented 8 years ago

Yes please to the .gitignore and test refactoring. :)

coveralls commented 8 years ago

Coverage Status

Changes Unknown when pulling b1beec0854f3fa0ac099db54faa63f0dd1552793 on hugovk:travis-ci into \ on araile:develop**.

ljcooke commented 8 years ago

I've made some changes so that the unit tests and coverage can also be run locally (using make test).

I'm happy to merge this in now. Thanks again!

hugovk commented 8 years ago

Good stuff, happy to help out!