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

Improved tests for get_filter_data & cache_filters module #39

Merged
merged 1 commit into from Aug 6, 2019

Conversation

jaladh-singhal
Copy link
Member

This PR is to improve the efficiency of tests I created for get_filter_data & cache_filters module (by increasing code coverage & reducing execution time). Following are the changes in their corresponding test modules:

  • test_get_filter_data:

    • get_filter_index() fetches the complete filter index & takes > 30s, so I limited the index length by passing Wavelength_Eff parameters to it, hence time reduced to ~7s only
    • Failing case for data_from_svo() is not covered, so I added a test for it
  • test_cache_filters:
    Cache reading functions (load_filter_index & load_transmission_data) are being covered completely but not the cache writing functions (download_filter_data & update_filter_data) since they have huge data dependencies. They are internally calling get_filter_index() (without parameters) to get entire dataset and hence it didn't seem feasible to test them directly so we were testing only the helper functions they are calling.
    Now in this PR I've resolved this limitation by using pytest monkeypatch fixture to mock that huge dependency to a very shorter one (by using wavelength range parameters). Therefore we can now actually test the main download & update functions, and their helpers functions will get implicitly tested. This has also increased coverage for cache_filters module significantly (from ~67% to 95%)!
    So in this PR, you'll find some major changes in the tests for cache writing functions which has even resulted several other benefits:

    • Reduced dependency on SVO by eliminating the direct specification of filter ids list which are prone to change. Instead of it, now we are using wavelength range to iterate over filters.
    • Isolated the directory from which cache is read and into which cache is written, which should be logically different to prevent changing the saved test data.

Also I've included enough comments to explain the code I added, to ensure that the tests remain maintainable.

@jaladh-singhal
Copy link
Member Author

The build is failing because I forgot that #37 is not yet merged so update_filter_data() is not recognized. I've tested this locally, once that PR is merged, we can re-run the build and see if it passes!

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

2 participants