os-climate / hazard

Onboarding, creation and transformation of climate hazard models for OS-Climate
Apache License 2.0
8 stars 12 forks source link

Failing tests #37

Closed emileten closed 7 months ago

emileten commented 7 months ago

@joemoorhouse as part of #36 I'd like first to make sure I can run tests locally. I am encountering two issues:

  1. I can't run the tests by following the CONTRIBUTING.md guide. More precisely, I do get past the pipenv environment creation, but nothing happens when I run the tox commands which the documentation points to, to run the tests. At any tox command I get this message
ROOT: No tox.ini or setup.cfg or pyproject.toml found, assuming empty tox.ini at /Users/emiletenezakis/devseed/os-climate-hazard
  1. I can circumvent this and run pytest directly. More precisely, from the root of the repo pytest src/test. However many tests fail (see the summary of the log below. I can share more details if useful).

Do these tests run successfully on your side ? If so, can you share your set up if it is at all different from mine (or the one described in the docs).

=========================== short test summary info ============================
FAILED src/test/test_drought_indicators.py::test_spei_indicator - KeyError: 'OSC_S3_ACCESS_KEY_DEV'
FAILED src/test/test_drought_indicators.py::test_check_result - KeyError: 'OSC_S3_ACCESS_KEY_DEV'
FAILED src/test/test_drought_indicators.py::test_doc_store - ValueError: environment variable OSC_S3_ACCESS_KEY_DEV not present
FAILED src/test/test_tile_creation.py::test_xarray_writing - FileNotFoundError: No such file or directory: '<zarr.storage.DirectoryStore...
FAILED src/test/test_tile_creation.py::test_map_tiles_from_model - KeyError: 'inundation/wri/v2/inunriver_historical_000000000WATCH_1980'
FAILED src/test/test_tile_creation.py::test_convert_tiles - KeyError: 'OSC_S3_BUCKET'
FAILED src/test/test_wind_onboarding.py::test_wind_onboarding - rasterio.errors.RasterioIOError: /Users/emiletenezakis/devseed/os-climate-h...
============ 7 failed, 11 passed, 14 skipped, 40 warnings in 4.04s =============

Thanks !

joemoorhouse commented 7 months ago

Hi @emileten,

Certainly some of those are expected to fail without credentials set up: there are some tests which need S3 access; others are mocked. But as I mentioned, the hazard repo needs to be brought into line with the other repos and Linux Foundation best-practice (we don't want 'hazard' to have a double meaning :)) and think it's best to do that right now - since you will also need the CI/CD pipeline. I just discussed this with @ModeSevenIndustrialSolutions. I'll make the most pressing changes tomorrow/Friday this week:

Does all that sound OK for you? Thanks, Joe

emileten commented 7 months ago

Thanks @joemoorhouse yes that makes sense!

In the meanwhile, is there any way for me to run these tests locally ? From the logs it looks like setting up my AWS credentials in a standard way with no particular permissions isn't going to work (the code seems to be looking for some specific environment vars for example).

No worries if not, but if that's easy I might as well do it

emileten commented 7 months ago

@joemoorhouse I think I am able to run the tests locally now. Question though, do any of these tests now actually download any data from the OS-C S3 buckets ? I see a lot of those that actually do I/O with S3 are skipped.

joemoorhouse commented 7 months ago

Hi @emileten,

No, currently the tests are all mocked - any which actually move data around are skipped for CI purposes. My assumption up until now has been that we don't want our CI tests to be reading and writing data to S3. But certainly one to revisit. There are some tests very hard to do without actually reading/writing something. @ModeSevenIndustrialSolutions, @MichaelTiemann, @redmikhail might be interested in that discussion.

Thanks, Joe