WattTime / pyiso

Python client libraries for ISO and other power grid data sources.
http://pyiso.readthedocs.org/
Other
237 stars 110 forks source link

Separate unit tests from integration tests #130

Closed r24mille closed 6 years ago

r24mille commented 6 years ago

Currently, there are many tests in the test directory that call external resources, rather than mocking requests/responses. Some also require environment variables (e.g. setting EIA_KEY in test_eia.py). This is causing issues in Travis and we're losing the value from existing continuous integration work that has been done.

I propose the following changes:

  1. The codebase seems to follow a tests outside application code directory structure, which is fine. In line with this Stack Overflow thread I'd further organize directories into:
    pyiso
    + --- tests
    |       + --- unit
    |       |         | test_base.py
    |       |         | test_client_one.py
    |       |         | test_client_two.py
    |       |
    |       + --- integration 
    |                 | integration_test_client_one.py
    |                 | integration_test_client_two.py
    |       
    + --- pyiso
          | base.py
          | client_one.py
          | client_two.py
  2. Fix travis.yml so that the build server only runs unit tests when pull requests are opened. I believe this is just changing the [nosetests](https://nose.readthedocs.io/en/latest/usage.html) script on line 26 to something like:
    script: nosetests -w ./tests/unit

This only leaves one question: What automated process should run the integration tests? Should the maintainers just be responsible for running integration tests before formalizing a release? Should the integration tests just be there for developers to run out-of-band so they can be confident in their code before opening pull requests?

r24mille commented 6 years ago

Hmm... it seems #131 auto-closed this issue when it got merged in (?) Anyway, it may be useful to split the work up into multiple issues or re-open this issue. I don't have permissions, since I'm not a collaborator.

ajdonnison commented 6 years ago

Reopening as it needs more work.