search

ucl-exoplanets / ExoTETHyS

GNU General Public License v3.0
17 stars 3 forks source link

Tests #10

Closed steven-murray closed 4 years ago

steven-murray commented 4 years ago

Part of JOSS review:

There are no explicit tests for ExoTETHys. Automated tests (using eg. pytest) are very helpful in catching coding errors quickly, and also for helping new users know that their installation is working as intended.

gmorello commented 4 years ago

In principle, the examples are intended to be the tests. Some of them should work as they are, others require hard-coded paths, which is also something that should be tested. Do you have specific suggestions on how to improve these tests?

steven-murray commented 4 years ago

For your own sanity, it is useful to have this process automated. If you have an idea of what the output should be for these examples, you could literally just create a tests directory, write a test_sail.py module, and have a function per example. This can be set up to run automatically on every push at TravisCI.

gmorello commented 4 years ago

I created the test_sail.py. Could you check if that is now fine? (Meanwhile I will create the test_trip.py)

steven-murray commented 4 years ago

That file looks great! Are you going to try running a "full" example as a test as well? Do you need help setting up travis ci?

gmorello commented 4 years ago

Yes, for now I get the error "no environment variables set". It appears that the error originates from importing libraries which are not installed by default, e.g. scipy.

steven-murray commented 4 years ago

Can you point me to the travis log? You should probably add py3.6 and py3.7 to your test matrix as well.

gmorello commented 4 years ago

Can you see it? https://travis-ci.org/ucl-exoplanets/ExoTETHyS/jobs/620680306?utm_medium=notification&utm_source=github_status

steven-murray commented 4 years ago

Yes, thank you. It looks like the issue was related to #5 , which has now been fixed. However, the build is failing on pypy when it tries to build scipy. I've never used pypi before, so I'm not sure how much I can help on that. It looks like you might have to install blas via apt on travis.

gmorello commented 4 years ago

Thank you very much. It passed the last test by installing blas and lapack in the travis file. But I still need to add some tests for the trip subpackage.

gmorello commented 4 years ago

I think that tests are also complete now, and travis ci does not reveal any troubles. @steven-murray Running a full example in the tests sounds tricky, as it would require to read files outside the package anyway. But I am going to clarify which examples should work and which ones need to be edited in the README. I hope this issue can be closed as well.

steven-murray commented 4 years ago

That sounds good. I'm happy for this to be closed, however I will say that it's quite fine to be reading files outside the package (so long as they're available to download in an automatic way, or if "mock" ones can be created in the test package). I'll leave that up to you. The note in the readme does sound important though!