Skip to content

Add unit tests for cppcheckdata.py#8707

Open
pfultz2 wants to merge 5 commits into
cppcheck-opensource:mainfrom
pfultz2:addon-unit-tests
Open

Add unit tests for cppcheckdata.py#8707
pfultz2 wants to merge 5 commits into
cppcheck-opensource:mainfrom
pfultz2:addon-unit-tests

Conversation

@pfultz2

@pfultz2 pfultz2 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

This adds unit tests for cppcheckdata.py which runs with pytest. It will also run these tests with cmake automatically if python is available, so this should run on the CI.

The unit tests did surface a couple of problems, but I didnt fix it this PR to keep the scope smaller. Instead I just marked the test with pytest.mark.xfail for now. I can do a follow up PR to fix these issues:

  1. Token.isBoolean is dead code. Tokenizer::dump (lib/tokenize.cpp:6179) checks tok->isName() before isBoolean(), and eBoolean tokens count as names (lib/token.h:397), so true/false are always dumped as type="name" and the type="boolean" branch at tokenize.cpp:6199 is unreachable for them(I see there is TODO: "true"/"false" aren't really a name...)
  2. Line suppressions match every line. The final "other suppression" fallback in Suppression.isMatch() (cppcheckdata.py:1007) doesn't check that lineNumber is unset, so a suppression for line 5 also matches line 6. The fix would be a one-line self.lineNumber is None guard.
  3. In C++ Token::Match, !!x also matches when there is no token at all (null), but Python match() fails when the token list ends: match(last_brace, '} !!x') returns false.

@danmar

danmar commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

I didn't review it precisely but it sounds good to me 👍

@pfultz2

pfultz2 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

The timeout is unrelated to this PR.

@danmar danmar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it but well I would prefer if @firewave also took a look..
I see nothing bad in the code.

@danmar

danmar commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

The timeout is unrelated to this PR.

I think the timeout value was chosen "somewhat aggressively" by intention so that it will timeout sometimes. And we will see if it starts to fail more. @firewave do you have opinions about that change?

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