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 clang-tidy warnings for static fields created by doctest #159

Merged
merged 3 commits into from
Nov 3, 2018
Merged

Remove clang-tidy warnings for static fields created by doctest #159

merged 3 commits into from
Nov 3, 2018

Conversation

rantasub
Copy link
Contributor

@rantasub rantasub commented Nov 3, 2018

Description

When writing unit tests with doctest, clang-tidy raises a warning at each invocation of the TEST_CASE() or SCENARIO() macro, explicitly fuchsia-statically-constructed-objects and cert-err58-cpp for my current version of clang-tidy. The root cause is the intended creation of static variables by the framework. As these warnings are not a consequence of the user written code and there are plenty of them for a big test suite, they should be disabled.

This pull request adds an explicit suppression for these two tests at the responsible line in the implementation.

GitHub Issues

The warning cert-err58-cpp was already discussed in issue #115. The solution provided there however requires some action by the user.

onqtam and others added 3 commits October 24, 2018 17:47
The warnings

- fuchsia-statically-constructed-objects
- cert-err58-cpp

are disabled for the creation of static fields within the
DOCTEST_GLOBAL_NO_WARNINGS macro. This removes these clang-tidy
warnings in user written unit tests at invocation of the
TEST_SUITE and related macros.
@onqtam
Copy link
Member

onqtam commented Nov 3, 2018

A comment for suppression at the end of a macro works when the macro is expanded elsewhere through a few macro indirections? Nice!

Thanks for the PR!

I'll probably get to publishing version 2.0.2 (so this will go from the dev branch into master) in a few weeks...

@onqtam onqtam merged commit 288e34f into doctest:dev Nov 3, 2018
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