tests: increase coverage for commaSeparatedReturn (negative case) [#13869] #7746
Conversation
…ntainers Signed-off-by: Aryan Rahar <aryanrahar1@gmail.com>
Signed-off-by: Aryan Rahar <aryanrahar1@gmail.com>
…a in return) [#13869] Signed-off-by: Aryan Rahar <aryanrahar1@gmail.com>
test/teststl.cpp
Outdated
|
|
||
| TEST_CASE(checkKnownEmptyContainer); | ||
| TEST_CASE(checkMutexes); | ||
| TEST_CASE(commaSeparatedReturn_no_fp); |
There was a problem hiding this comment.
it seems misplaced where are our other commaSeparatedReturn tests?
my hope is that if we put related tests together it's possible to see overlaps and gaps ..
test/teststl.cpp
Outdated
| "}\n"); | ||
| ASSERT_EQUALS("", errout_str()); | ||
| } | ||
| void commaSeparatedReturn_no_fp() |
There was a problem hiding this comment.
this code is not formatted properly. Either run the runformat script or there is a diff from the "format" action that you can download and apply.
Signed-off-by: Aryan Rahar <aryanrahar1@gmail.com>
…se; no comma in return) Signed-off-by: Aryan Rahar <aryanrahar1@gmail.com>
|
Moved the test to test/testother.cpp, registered it next to checkCommaSeparatedReturn, and formatted with .uncrustify.cfg. |
|
|
is there any issue? |
|
https://trac.cppcheck.net/ticket/13869 says that the check is currently disabled anyway? |
|
The point was rather to have test which trigger the warning although negative can be good as well. We mostly add them if false positives are encountered (sometimes already during development). But as pointed out, this is a check which is disabled so it is rather pointless to add it. |
|
Thanks for the contribution but as the checker should be rewritten I think it does not make sense to add this test code. Can you please look at rewriting the checker?
this can be implemented relatively easily. if there is just some calculations and no function calls, assignments, increments nor decrements before the comma then warn. |
|
I close this PR.. please feel free to open a new PR that fixes https://trac.cppcheck.net/ticket/5076 by rewriting the checker.. |
|
Rewriting a checker seems too much of an ask for a first-time contributor (well, not even me would be able to rewrite it). It would make more sense to look at the other tickets about missing test coverage for checks. |
4 participants
Adds a negative test next to existing
checkCommaSeparatedReturncoverage:returnwith no comma operator → expect no diagnosticPlacement:
test/testother.cpp, registered inTestOther::run()aftercheckCommaSeparatedReturn.Formatting: applied with
.uncrustify.cfg.Earlier version accidentally triggered
constStatement; this version avoids unrelated warnings.All tests pass locally.