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

Add unit tests #8

Merged
merged 6 commits into from
Apr 19, 2016
Merged

Add unit tests #8

merged 6 commits into from
Apr 19, 2016

Conversation

hugovk
Copy link
Contributor

@hugovk hugovk commented Apr 18, 2016

  • Just some simple unit tests to be going on with. This'll make it easier to add more in the future, and let's us know if it even runs on different Python versions. Also you see if future PRs break things.
  • Run unit tests on Travis CI. Python 2.6, 2.7, 3.2 3.3, 3.4, 3.5, PyPy and PyPy3 all passIt gives a build report like https://travis-ci.org/hugovk/see/builds/123890089
  • The tests are also run with coverage, which is at 66.67%, and is also sent to Coveralls so you get a report like https://coveralls.io/builds/5827201 where you can see which lines are covered or not.
  • If running tests locally, you can do python test_see.py without coverage; or coverage run --source=see ./test_see.py -v with coverage. The coverage report shows the results, or coverage html to generate local files in htmlcov/

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

@ljcooke
Copy link
Owner

ljcooke commented Apr 18, 2016

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:

  • I'll look into the services you linked. Travis seems to be a good choice.
  • 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.)
  • 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.
  • 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?)

@hugovk
Copy link
Contributor Author

hugovk commented Apr 18, 2016

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
Copy link
Owner

ljcooke commented Apr 18, 2016

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

@coveralls
Copy link

Coverage Status

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

@ljcooke
Copy link
Owner

ljcooke commented Apr 19, 2016

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!

@ljcooke ljcooke merged commit b1beec0 into ljcooke:develop Apr 19, 2016
ljcooke added a commit that referenced this pull request Apr 19, 2016
@hugovk hugovk deleted the travis-ci branch April 20, 2016 05:57
@hugovk
Copy link
Contributor Author

hugovk commented Apr 20, 2016

Good stuff, happy to help out!

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

Successfully merging this pull request may close these issues.

None yet

3 participants