Conversation
| ------- | ||
|
|
||
| Before submitting any changes please make sure all tests and checks are passed, pylint doesn't show warnings on new code, and fill out the Pull Request template. If you are a new contributor, and the template is confusing, feel free to submit the PR and we will help you iron it out! On how to run or add a new test, please see [test/README.md](test/README.md). | ||
| Before submitting any changes please make sure all tests and checks are passed. |
There was a problem hiding this comment.
I'm sure, before running anything we must crealy describe how to initialize the environment.
See Development Environment above
| # This script currently assumes that the user have set up their environment. | ||
| # This means users using bazelisk have set their bazel version. | ||
| # Either with the environment variable: USE_BAZEL_VERSION | ||
| # or with the .bazelversion file. |
There was a problem hiding this comment.
Hm... discussable...
Maybe we should setup correct environment in this script?
There was a problem hiding this comment.
Set up through program arguments/environmental variables? I understand that our clientbase have their own bazel and CodeChecker distributions, so we can't generalize too much here.
This sounds like a reasonable compromise:
USE_BAZEL_VERSION=8.5.0 CODECHECKER_BIN_PATH=/path/to/codechecker/25 run_tests.sh
There was a problem hiding this comment.
This is about Development Environment not Execution Environment since we run tests, right?
Here we have to find common ground, common environment that works for all possible contributors, starting from you and me, but extend to anyone. Which already means at least different versions of Ubuntu and RedHat, different version sets of Bazel and Clang etc.
That's exactly why I suggested Mise and/or Micromamba to solve this problem.
Both are still not ideal, but at least something. Maybe some alternatives are possible too.
(BTW, native bazel dependency manager may work, but we did not try)
There was a problem hiding this comment.
We could very well set up the python virtual environment for testing in this script, but I would be hesitant to also do that for the llvm toolchain (maybe thats where im wrong?).
My first idea is whether a bazel provided llvm toolchain exist and can we use that?
My experience is that most of these crashes should be due to diagtool not being found by clang. (You can check the codechecker log for the failing analyzed file to confirm this, even though the plist files arent getting created the log still does)
There was a problem hiding this comment.
We could very well set up the python virtual environment for testing in this script, but I would be hesitant to also do that for the llvm toolchain (maybe thats where im wrong?).
Right, unfortunately, venv (virtual environment) is not sufficient, that's exactly why I suggested mise and/or micromamba.
My first idea is whether a bazel provided llvm toolchain exist and can we use that?
Yes, it is theoretically possible!
Practically... I dont know, it might be difficult to find llvm package that we could use.
My experience is that most of these crashes should be due to diagtool not being found by clang. (You can check the codechecker log for the failing analyzed file to confirm this, even though the plist files arent getting created the log still does)
There are so many different failures depending on versions and environments, so did not check them.
We need to fix the environment first, then set of tests and the way we run them.
After that I can report and investigate fails.
I chose this approach, since we cannot expect users to run our programs inside a docker container or a micromamba/mise managed environment.
|
I have investigated the possibility of using the bazel module: https://github.com/bazel-contrib/toolchains_llvm So instead, I tried this different approach, setting up the environment "manually". |
Why:
The requirements for running tests are not well defined.
This patch aims to define which targets should be run for testing.
This patch does not define how the user should set up their environment, only how the tests should be run. That is a todo for the next patch.
What:
CONTRIBUTING.mdto specify which tests must be runAddresses:
none?