refs #12358 - CI-unixish.yml: added step which checks for AST and ValueFlow changes by firewave · Pull Request #7800 · cppcheck-opensource/cppcheck

Spoiler alert: I start to ramble in the later part of this comment because those realization just started coming in and I didn't feel like re-doing the whole post. Sorry about that.

Can you implement this test somehow else. Maybe generate selfcheck.exp automatically based on cppcheck git head or whatever the PR target branch is. how about just showing the diff in a PR comment so it can be resolved/dismissed..

The problem is that you need some kind of baseline. That means you need to build and run the previous version to generate that. We could store the binary and/or the output and pull that but I do want that dependency from a workflow into a previously build job (it might also not be reliable - we had issues with that within the same workflow in the same build). It would also increase the runtime and complexity of workflow if we have to pull and build the current version is it based off (and that also needs to consider branches and fork - maybe even more).

I thought about doing this as scheduled job (like IWYU or CSA) but that was with a much bigger scope (simplecpp is a more nicer contained example to use). That still would require the generation of the baseline and it would only be triggered periodically. And scheduling it off on each PR felt too disjoint and might only accumulate reports nobody even wants to check (see Coverity). The scheduled CSA is already piling up findings (it is only scheduled because it is too slow). IWYU is separate because it still has too many false positives (in simplecpp I was able to enable misc-include-cleaner from clang-tidy as it has a much smaller and manageable scope). And if those changes were made by one-off contributors those things might never be fixed because don't even know what the intended behavior is supposed to be.

I also thought about showing it as a comment instead without failing the build (IWYU does that) but that still doesn't solve the baseline. And not failing the build or having it disjoint will

I see it just like the results of any other test. If I change the output I need to adjust tests. I also feel this would make some things easier to track.

And I just realized that fail the builds on linting stuff like formatting. That has no impact on the behavior and we require immediate mitigation. You could argue that we just do a clean up step when the release is done - but since it is enforced in the CI we don't have to. It is kind of similar to the translations files - that should be kept in line so they are actually correct. But we have it disabled since there is currently too much noise from unrelated changes. That does not apply in this case since you only need to adjust the file if you actually did something to the ValueFlow. It does not just change because you fixed a random bug.