-
Notifications
You must be signed in to change notification settings - Fork 436
[bazel] Add tests from codechecker_bazel to the analyzer repository #4514
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
base: master
Are you sure you want to change the base?
Conversation
0802ea5 to
611d239
Compare
65b2319 to
c821ee8
Compare
This PR adds some tests that utilize the rules found in https://github.com/Ericsson/codechecker_bazel. The tests are added under the analyzer directory, as these are plain analysis runs, only invoked through bazel. These are the things to highlight: * codechecker_bazel only supports bazel 6. If you have seen/heard of a bazel feature you are missing form this commit, its likely missing from bazel 6 is well. * Bazel creates a lot of temporary files and directories, all of these need to ignored from .gitignore and pytest. * Bazel wipes the environment. This means that for the eventual CodeChecker call, the proper environment has to be reassembled again. This is mostly done via the bazel rules, but I had a lot of trouble with the wrapper script finding CodeChecker in the tests. Becuase of this, I manually prepend the CodeChecker bin folder onto PATH. * The actual tests (which were copy-pasted from the codechecker_bazel repository) were made disregarding gcc, cppcheck and infer. These things were adjusted in the BUILD file (only specific analyzers were enabled). * The lock file is usually not ignored. Not too sure on that. * The WORKSPACE file is likely among the most interesting -- this is how you can use the codechecker bazel rules in your own codebase. Note how we don't clone the main branch of the codechecker_bazel repository for now, but I already have plenty of PRs about to be merged in that repository as well, so this will change back to the main branch soon. * In the __init__.py file I tried to prepend CodeChecker to PATH, but it never worked out. * test_bazel_test_clang_ctu_fail is still failing. Seems like CTU is not working yet. For the time being (because this PR has been stagnating for long enough) I just commented it out. * The patch depends on Ericsson#4558.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's really great that you made this test working! Congrats for that!
I see that these tests are coming from the https://github.com/Ericsson/codechecker_bazel/
repo.
These tests are testing the bazel rules laid out in the bzl files in that repo: codechecker_config, codechecker, codechecker_test, code_checker_test, clang_tidy_test etc.
I think a much more natural place for this github action would be in the codechecker_bazel repo, beacause they are essentially testing the implementation of those rules.
It would look odd, to need to modify these tests in the codechecker repo, when an action is renamed or rewritten in the other bazel repo.
The codechekcer bazel repo has CodeChecker as its dependency and not the other way around.
I understand the motivation for having a regression test in codechecker as codechecker changes more frequently, but still I would not duplicate test code this way.
Since codechecker command line is fairly stable (or backward compatible), I would think these tests would not break frequently.
Vscode plugin (which also depends on CodeChecker command line IF) tests are also not placed in the CodeChecker repo, rather in the vscode plugin repo.
So I think we should follow a similar login there.
What do you think?
|
Moved tests here. |
This PR adds some tests that utilize the rules found in
https://github.com/Ericsson/codechecker_bazel. The tests are added under
the analyzer directory, as these are plain analysis runs, only invoked
through bazel.
These are the things to highlight:
codechecker_bazel only supports bazel 6. If you have seen/heard of a
bazel feature you are missing form this commit, its likely missing
from bazel 6 is well.
Bazel creates a lot of temporary files and directories, all of these
need to ignored from .gitignore and pytest.
Bazel wipes the environment. This means that for the eventual
CodeChecker call, the proper environment has to be reassembled again.
This is mostly done via the bazel rules, but I had a lot of trouble
with the wrapper script finding CodeChecker in the tests. Becuase of
this, I manually prepend the CodeChecker bin folder onto PATH.
The actual tests (which were copy-pasted from the codechecker_bazel
repository) were made disregarding gcc, cppcheck and infer. These
things were adjusted in the BUILD file (only specific analyzers were
enabled).
The lock file is usually not ignored. Not too sure on that.
The WORKSPACE file is likely among the most interesting -- this is how
you can use the codechecker bazel rules in your own codebase. Note how
we don't clone the main branch of the codechecker_bazel repository for
now, but I already have plenty of PRs about to be merged in that
repository as well, so this will change back to the main branch soon.
In the init.py file I tried to prepend CodeChecker to PATH, but it
never worked out.
test_bazel_test_clang_ctu_fail is still failing. Seems like CTU is not
working yet. For the time being (because this PR has been stagnating
for long enough) I just commented it out.
The patch depends on [bugfix] Pass the correct interpreter from bin/CodeChecker to the analyzers #4558.