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

Coverage of the test suite (JOSS Review) #42

Closed
shreyasbapat opened this issue Jan 14, 2021 · 5 comments
Closed

Coverage of the test suite (JOSS Review) #42

shreyasbapat opened this issue Jan 14, 2021 · 5 comments

Comments

@shreyasbapat
Copy link

The test suite has two major issues:

  1. There's no logical separation to check which module is being tested.
  2. The test suite seems very small. I am curious if that is getting you enough coverage to check everything. However, this is not a necessary requirement for the JOSS Review, but it would be great to understand how much of the code is tested.

openjournals/joss-reviews#2854

@rmorgan10
Copy link
Contributor

@shreyasbapat I've now introduced a logical organization in the test suite with commits 3e9e4dc and 6b1ad6d

When each separate testing module is run, a detailed description of what is being tested is now printed to the screen. You can see the output of the test suite in the updated Run_Tests notebook: https://github.com/deepskies/deeplenstronomy/blob/master/test/Run_Tests.ipynb

@rmorgan10
Copy link
Contributor

rmorgan10 commented Jan 20, 2021

Overall, coverage is difficult to assess for this particular software. A lot of the internal functions are drawing from distributions using random, numpy, and scipy or simulating images with lenstronomy, so those parts of my software that have been tested in those particular packages.

What the tests I include in this suite cover is mapping the values in the input configuration file to the outputs of the simulation. Thus, by verifying that the generated simulations match expectations in terms of dataset size, attributes, objects and configurations included, the inclusion of time-series information, and other parameters, the testing suite here accomplishes that goal.

Also, I should mention that the deeplenstronomy.check module serves to automatically check that all user input (in the configuration file) is valid. Therefore, with the test suite testing that correct inputs map to correct outputs and the check module assuring that inputs are correct, the process from start to finish is validated.

Please let me know if you have any feedback! Thanks!

@rmorgan10
Copy link
Contributor

@shreyasbapat Can I go ahead and close this issue?

@shreyasbapat
Copy link
Author

Sorry for not putting a comment here. Sure you can close this. But I will still encourage to have a proper coverage report handy later sometime.

@rmorgan10
Copy link
Contributor

Sounds great, thanks for the review!

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

No branches or pull requests

2 participants