Skip to content

Improve C++ Test Coverage#920

Merged
rapids-bot[bot] merged 36 commits intorapidsai:branch-22.02from
harrism:fea-improve-codecov
Dec 1, 2021
Merged

Improve C++ Test Coverage#920
rapids-bot[bot] merged 36 commits intorapidsai:branch-22.02from
harrism:fea-improve-codecov

Conversation

@harrism
Copy link
Member

@harrism harrism commented Nov 17, 2021

Now that #905 added C++ code coverage support, this PR improves test coverage. Much of the improvement comes from adding a new adaptor_test.cpp which generically tests the common functions of all 7 adaptor types. Small test additions improve coverage of many other files as well. At least one typo bug was uncovered and fixed.

If you build with -DCODE_COVERAGE=ON -DRMM_LOGGING_LEVEL=TRACE, overall code coverage is now 99.5% and all but one file has 100% coverage. That one file, arena.hpp is undergoing work concurrent to this PR, and improvement to 100% requires additional testing that might be best undertaken by @rongou, so I will leave it.

image

@harrism harrism added tests Related to unit tests non-breaking Non-breaking change improvement Improvement / enhancement to an existing function labels Nov 17, 2021
@harrism harrism self-assigned this Nov 17, 2021
@github-actions github-actions bot added CMake cpp Pertains to C++ code labels Nov 17, 2021
@harrism harrism marked this pull request as ready for review November 18, 2021 03:39
@harrism harrism requested review from a team as code owners November 18, 2021 03:39
@harrism harrism requested review from cwharris and rongou November 18, 2021 03:39
@harrism
Copy link
Member Author

harrism commented Nov 23, 2021

Now up to 99% coverage without whitebox testing (and explicit instantiations uncovered more untested code, which I've fixed). I have a couple more things to try to fix a few more uncovered functions.

@harrism harrism added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Nov 24, 2021
@harrism
Copy link
Member Author

harrism commented Nov 24, 2021

Because a lot has been added since the reviewers approved, I'm rerequesting reviews.

@harrism
Copy link
Member Author

harrism commented Dec 1, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit d52c365 into rapidsai:branch-22.02 Dec 1, 2021
rapids-bot bot pushed a commit that referenced this pull request Dec 6, 2021
Fixes #928.

#920 introduced a new test for the `tracking_resource_adaptor::log_outstanding_allocations`, but that function only logs when `SPDLOG_ACTIVE_LEVEL <= SPDLOG_LEVEL_DEBUG`, so the test needs to be gated on that condition as well.

Authors:
  - Mark Harris (https://github.com/harrism)

Approvers:
  - Rong Ou (https://github.com/rongou)
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Bradley Dice (https://github.com/bdice)

URL: #929
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5 - Ready to Merge Testing and reviews complete, ready to merge CMake cpp Pertains to C++ code improvement Improvement / enhancement to an existing function non-breaking Non-breaking change tests Related to unit tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants