-
Notifications
You must be signed in to change notification settings - Fork 3
Add a small test project to run codechecker_bazel on #16
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
Conversation
.github/workflows/test.yml
Outdated
| - name: Run Bazel CodeChecker | ||
| run: | | ||
| cd yaml-cpp | ||
| bazel build :codechecker_test_yaml-cpp |
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.
We should likely use bazel test. Also, we added two new targets, codechecker_test_yaml-cpp and code_checker_test_yaml-cpp, but are only testing the former.
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.
Running it with bazel test produces errors (despite filling the log with the success message of CodeChecker), thats why I have been using bazel build.
nettle
left a comment
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.
Good approach! But I think this project (yaml-cpp) should not be the only one. I would suggest adding this project as one-of-many, like putting files into sub directories with patches and test launchers etc
.github/workflows/test.yml
Outdated
| - name: Run Monolithic Bazel CodeChecker | ||
| run: | | ||
| cd test_project | ||
| bazel build :codechecker_test |
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.
I think its still okay to call bazel test here, just negate the return value. Preferably also add a coment as to why the return value needs negating (because there are findings by CodeChecker).
Szelethus
left a comment
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.
Please change "small project" to the name of the project consistently, as we will add more than one project. Otherwise LGTM!
nettle
left a comment
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.
Good approach! But I think this project (yaml-cpp) should not be the only one. I would suggest adding this project as one-of-many, like putting files into sub directories with patches and test launchers etc
6d23639 to
e9dde24
Compare
|
Let's agree on the structure first, see #22 |
2962a92 to
03ac739
Compare
|
Changed to the structure we agreed on in: #22 The composite action file's name must be |
b2d5bab to
2a29b1b
Compare
|
This PR is getting too big, so I'm leaving it here as a template. |
This way the thest project is easily replacable
Unsure if its a good idea!
Created a ci job (small_test), to run the codechecker Bazel rule on a tiny Bazel project (yaml-cpp)
The installation of static analyzers and CodeChecker, and such have been moved to actions/env_setup/action.yml, to avoid repeating code.