Catch2 icon indicating copy to clipboard operation
Catch2 copied to clipboard

adds clang-tidy warning suppressions for `CHECK` and `TEST_CASE`

Open cjdb opened this issue 3 years ago • 1 comments

Both CHECK and TEST_CASE were giving warnings at the point of use. This commit makes it so that they do not (for now).

Checks disabled:

  • cppcoreguidelines-avoid-do-while (for CHECK)
  • cppcoreguidelines-avoid-non-const-global-variables (for TEST_CASE)

cjdb avatar Nov 30 '22 05:11 cjdb

Codecov Report

Merging #2585 (986c780) into devel (2d7be1f) will not change coverage. The diff coverage is 100.00%.

:exclamation: Current head 986c780 differs from pull request most recent head 142703e. Consider uploading reports for the commit 142703e to get more accurate results

@@           Coverage Diff           @@
##            devel    #2585   +/-   ##
=======================================
  Coverage   91.09%   91.09%           
=======================================
  Files         187      187           
  Lines        7622     7622           
=======================================
  Hits         6943     6943           
  Misses        679      679           

codecov[bot] avatar Nov 30 '22 06:11 codecov[bot]

I was planning on creating the exact same pull request. I did notice this one was not applied (yet), could this please be done. I understand the argument to rather not do it, but please understand when user would like to follow this guideline on their own code, it is annoying that Catch2 gives rise to this warning, and as such either we need to patch it locally or we need to suppress it in the project/unit tests, which is then also suppressing in our own code.

killerbot242 avatar Apr 22 '23 06:04 killerbot242

I've given this some more thought:

  1. I am willing to merge code changes to keep clang-tidy happy. I prefer code changes to suppressions, see e.g. the change to constify the registration vars.
  2. I am not willing to entertain cppcoreguidelines-avoid-do-while. Set your checker to ignore do {} while in macros.

horenmar avatar Jun 18 '23 13:06 horenmar