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

Feature/move tests to integration #709

Merged
merged 11 commits into from
May 12, 2023
Merged

Conversation

NicolasColombi
Copy link
Collaborator

@NicolasColombi NicolasColombi commented Apr 28, 2023

Changes proposed in this PR:

Move lazy unittest which loads data multiple time to the integration folder.

List of tests moved:

  • climada/util/test/test_lines_polys_handler/test_calc_geom_impact_polys (~ 15.5 sec)
  • climada/hazard/test/test_storm_europe/test_icon_read (~ 4.5 sec)
  • climada/hazard/test/test_storm_europe/test_from_footprints (~ 6 sec)
  • climada/hazard/test/test_ibtracs_with_basin (~6 sec)
  • climada/hazard/test/test_cutoff_tracks (~5 sec)
  • climada/hazard/test/test_base/test_write_read_pass (4.5 sec)
  • climada/hazard/test/test_base/test_raster_to_vector_pass (3.5 sec)
  • climada/hazard/test/test_base/test_reproject_raster_pass (3.6 sec)

This PR decrease the cost of unittest by around 51s. All tests are placed in the integration folder either under test_hazard.py under a new classes, or in the new file test_util.py.

The above tests share in common the following characteristics:

  • they all load data (multiple times for the most of them)
  • almost all of them call other climada functions than the one they are testing

PR Author Checklist

PR Reviewer Checklist

Move test: test_calc_geom_impact_polys situated in:
climada/util/test/test_lines_polys_handler.py:289

to: climada/test/test_util.py

the test takes around 16 seconds
Tests moved:

1) climada/hazard/test/test_storm_europe/test_icon_read (~ 4.5 sec)
2 ) climada/hazard/test/test_storm_europe/test_from_footprints (~ 6 sec)

Destination:

1) climada/test/test_hazard/test_icon_read
2) climada/test/test_hazard/test_from_footprints
Move test:

1) climada/hazard/test/test_ibtracks_with_basin (~6 sec)
2) climada/hazard/test/test_cutoff_tracks (~5 sec)

Destination:

1) climada/test/test_hazard/test_ibtracks_with_basin
2)climada/test/test_hazard/test_cutoff_tracks
Move tests:

1) climada/hazard/test/test_base/test_write_read_pass  (4.5 sec)
2)climada/hazard/test/test_base/test_raster_to_vector_pass (3.5 sec)
3)climada/hazard/test/test_base/test_reproject_raster_pass (3.6 sec)

Destination:

1) climada/test/test_hazard/test_write_read_pass
2) climada/test/test_hazard/test_raster_to_vector_pass
3) climada/test/test_hazard/test_reproject_raster_pass
@NicolasColombi NicolasColombi removed their assignment Apr 28, 2023
@NicolasColombi NicolasColombi marked this pull request as ready for review May 1, 2023 19:43
@chahank chahank requested a review from peanutfun May 3, 2023 09:53
@peanutfun
Copy link
Member

peanutfun commented May 4, 2023

Excellent initiative, @NicolasColombi, thanks a lot! 🙌

I used black to format the new file and the inserted code in test_hazard.py. I also cut the duplicated code from test_util.py by simply importing the stuff from climada.util.test.test_lines_polys_handler. This seems to work fine.

On my system, one test of test_hazard.py fails. This might be related to #678? As Jenkins does not run integration tests on pull requests, I do not know if the test would fail there as well. @NicolasColombi, does the test run through at your end?

$ python climada/test/test_hazard.py

======================================================================
ERROR: test_icon_read (__main__.TestStormEurope)
test reading from icon grib
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/ldr.riedel/coding/climada_python/climada/test/test_hazard.py", line 184, in test_icon_read
    haz = StormEurope.from_icon_grib(
  File "/Users/ldr.riedel/coding/climada_python/climada/hazard/storm_europe.py", line 506, in from_icon_grib
    intensity=sparse.csr_matrix(stacked.gust.T),
  File "/Users/ldr.riedel/mambaforge/envs/climada_env_3.9/lib/python3.9/site-packages/xarray/core/common.py", line 276, in __getattr__
    raise AttributeError(
AttributeError: 'Dataset' object has no attribute 'gust'

@NicolasColombi
Copy link
Collaborator Author

Hi @peanutfun, Thanks a lot for the review!! 😄 I run all the tests again and they pass, including test_icon_read. I guess you are right, it must be related to #678... Do we want to ask @chahank to run the test on his machine to see if it also fails ? Alternatively, we wait that #678 get fixed and then you can try again from yours @peanutfun :)

@peanutfun peanutfun merged commit 8379737 into develop May 12, 2023
3 checks passed
@emanuel-schmid emanuel-schmid deleted the feature/move_tests_to_integration branch May 25, 2023 08:41
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