Feat io tests #21
Feat io tests #21
Conversation
Co-authored-by: Gionata Ghiggi <[email protected]>
| "_get_info_from_filename", | ||
| autospec=True, | ||
| return_value={ | ||
| "product": "2A-CLIM", |
There was a problem hiding this comment.
I am not entirely sure I grasp correctly what it's going on here.
For whatever product below is passed in the download function below, _get_info_from_filename will return this dummy inexistent product information? Maybe we should mock this function and ensure that the result varies depending on the product info in the product.yaml file?
There was a problem hiding this comment.
Indeed, the same information is returned regardless of the filename. It seems the tests in this class were written just to make sure than the whole download_archive function is executed without issues, but there are no actual checks.
There was a problem hiding this comment.
Ok. Can you just add a short comment explaining that for the future me?
There was a problem hiding this comment.
Thanks for all the work ! We can discuss the last points in person this week maybe?
Also quickly resolve these issues ... https://app.codacy.com/gh/ghiggi/gpm_api/pullRequest?prid=12944041
For me it's fine then fine to merge this and then address the remaining issues in a separate PR ...
I see that CodeCov does not report anymore the coverage statistics ... and also Coveralls shows some issues.
So maybe let's merge, resolve the CI issues and then implement the last tests to the io module
| @@ -94,6 +103,7 @@ def remote_filepaths() -> Dict[str, Dict[str, Any]]: | |||
| "start_time": datetime.datetime(2020, 7, 5, 17, 0, 44), | |||
| "end_time": datetime.datetime(2020, 7, 5, 18, 33, 17), | |||
| "version": 7, | |||
| "granule_id": 36092, | |||
There was a problem hiding this comment.
Do you think this should be refactor or extended? Or generated from specific functions ?
There was a problem hiding this comment.
Each entry has a little difference to test various things. Maybe the entries could be indeed templated to look less copy pasted :)
There was a problem hiding this comment.
Note this down @sphamba and let's improve this at later stage ;)
| monkeypatch.setattr(conventions, "set_coordinates", mock_set_coordinates) | ||
|
|
||
| # Return a default dataset | ||
| da = xr.DataArray(np.random.rand(1, 1), dims=("other", "along_track")) |
There was a problem hiding this comment.
Maybe we should define here also a dataset for Grid data like IMERG that has ("time", "lat", "lon") columns?
And return the dataset according to the product?
Right now we test only for Orbit data ...
There was a problem hiding this comment.
The _prepare_test_finalize_dataset is not the only way I generate datasets for testing. Other dims are tested in test_finalize_dataset_reshaping, and it seemed to me that the rest of finalize did not depend on these dimensions. Tell me if you would still like to have the rest of finalize tested on Grid data
There was a problem hiding this comment.
Maybe it's worth still do this effort and have a test unit checking that the finalize_dataset functions works both from Orbit and Grid object.
So that if in future someone will enhance/modify the function, we ensure that the objects are processed correctly.
Summary
iochecks→ 100%fliter→ 100%info→ all used functionsfind:_get_all_daily_filepathsfor local, pps and ges discNote: coverage for
io/ges_discandio/ppswill be increased by testingio/download