BlueBrain / HighFive

HighFive - Header-only C++ HDF5 interface
https://bluebrain.github.io/HighFive/
Boost Software License 1.0
673 stars 159 forks source link

Remove duplicated tests. #1011

Closed 1uc closed 2 months ago

1uc commented 3 months ago

There's (at least) three attempts at testing read/write cycles for multi-dimensional arrays. The first is whatever is in tests_*multi_dims.cpp. Since it's partial, a second attempt was made: the first half of test_all_types.cpp. Since this was still partial and not quite DRY enough, a third attempt was made.

This PR removes the duplicated tests from the first and second attempt, one-by-one. Each commit removes one test. Or moves/reimplements a test if it's not yet covered by the third attempt.

There's a fourth attempt in test_high_five_base.cpp; but that's left for another PR.

1uc commented 3 months ago

If it helps reviewing, we could split out the commits that move code, e.g. compare_arrays into a separate PR. Then this one will be removing code and reimplementing the odd test; but no refactoring.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 84.81013% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 86.40%. Comparing base (5f3ded6) to head (2e4a41a).

Files Patch % Lines
tests/unit/compary_arrays.hpp 52.00% 12 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1011 +/- ## ========================================== - Coverage 86.81% 86.40% -0.41% ========================================== Files 100 102 +2 Lines 6081 5901 -180 ========================================== - Hits 5279 5099 -180 Misses 802 802 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

alkino commented 2 months ago

Nice to split test_boost and test_stl as test_opencv and test_xtensor already are. For the next time, maybe a test_eigen?

1uc commented 2 months ago

Interesting, I thought I'd split them out already, but yes, they can get split away too.