centraldedados / datacentral

Tools for generating portable data portals
58 stars 9 forks source link

Tests #22

Closed jberkel closed 8 years ago

jberkel commented 8 years ago

Noticed when working on Python 3: there's no way to automatically verify that everything is still working as expected. Could just be some simple smoke tests for now.

rlafuente commented 8 years ago

100% agree it's a problem not to have tests; however I have very little experience in creating tests :-/ I suppose the first part would be to verify that the right files are created, and maybe go from there.

What's your pick when it comes to Python testing packages?

jberkel commented 8 years ago

Don't have a strong preference, I'd just go with the built-in unittest / unittest2: https://docs.python.org/dev/library/unittest.html

rlafuente commented 8 years ago

Okay, so I introduced a minimal set of tests using Nose in 63f9c4107f9e5084445601ba0fcf67ff596edf2d

It should run with nosetests tests.py or make test.

jberkel commented 8 years ago

:+1: however you'll need to add nose to requirements.txt, otherwise it won't get installed

rlafuente commented 8 years ago

Hm, I was thinking whether it made sense to include nose in the required packages to install and run datacentral, since it only makes sense for developers. This thread has a couple answers arguing it's not necessary (or even desirable).

So I propose a compromise, which is explaining how to run the tests in the README -- I just went ahead and added the relevant lines in 712096f109d870fb3a19e69eb825eabeabaa732e.

If you think this works as a compromise, we can close this issue :-) (and perhaps create another for any tests that need to be written)

jberkel commented 8 years ago

ok, i'm not really familiar with python's packaging, but that's unfortunate, since developer dependencies / test code should be treated exactly like production code (in my opinion). anyway, let's close this for now.

rlafuente commented 8 years ago

No, reading this over I think you're right. Imposing minimal dependencies might be a case of over-engineering at this point, and right now developer convenience is the main concern. Nose is also pretty tiny, so I'm putting it in requirements.txt then -- thanks for the careful remarks :)