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

Remove duplicated tests. #1011

Merged
merged 18 commits into from
Jun 19, 2024
Merged

Remove duplicated tests. #1011

merged 18 commits into from
Jun 19, 2024

Conversation

1uc
Copy link
Collaborator

@1uc 1uc commented May 23, 2024

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
Copy link
Collaborator Author

1uc commented May 23, 2024

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.

Copy link

codecov bot commented May 23, 2024

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 ⚠️
Additional details and impacted files
@@            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              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@1uc 1uc force-pushed the 1uc/remove-multi_dims-tests branch from 2e64bb6 to 2e4a41a Compare May 24, 2024 05:40
@1uc 1uc marked this pull request as ready for review May 24, 2024 14:33
@alkino
Copy link
Member

alkino commented Jun 19, 2024

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

@alkino alkino merged commit bc1f1ef into master Jun 19, 2024
36 checks passed
@alkino alkino deleted the 1uc/remove-multi_dims-tests branch June 19, 2024 08:44
@1uc
Copy link
Collaborator Author

1uc commented Jun 19, 2024

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

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.

2 participants