Conversation
|
Hi @furtib and @Szelethus, |
There was a problem hiding this comment.
I like the idea, haven't thought of it in this way.
But why do it in a separate folder?
(minor note, due to how the CI is set up, it doesn't actually run these tests)
It also does not work on the Bazel 6 version, since we don't use the WORKSPACE file at all.
Just now I realized, tests is only used to disable the per-file test in yaml-cpp?
Let me show what I'm thinking of. (tomorrow...)
test/bazel/BUILD
Outdated
|
|
||
| foss_test( | ||
| name = "yaml-cpp", | ||
| url = "https://github.com/jbeder/yaml-cpp/archive/refs/tags/0.8.0.tar.gz", |
There was a problem hiding this comment.
I would prefer git hashes; they give us more options, and are more precise.
There was a problem hiding this comment.
Please dont prefer git hashes :)
Tags and releases are prices and also give timeline coordination plus certain level of responsibility.
While git hashes might be used as the last resort.
We will talk about it ;)
|
|
||
| # Resolve rules_codechecker path from the real script location | ||
| script_path = Path(os.path.realpath(__file__)) | ||
| self.rules_path = script_path.parent.parent.parent |
There was a problem hiding this comment.
I think this is more readable like this.
| self.rules_path = script_path.parent.parent.parent | |
| self.rules_path = script_path.parents[2] |
| ["chmod", "-R", "u+w", str(self.work_dir)], | ||
| capture_output=True, | ||
| ) | ||
| shutil.rmtree(self.work_dir, ignore_errors=True) |
There was a problem hiding this comment.
I think we don't need this; Bazel will remove the sandbox at the end of the run.
There was a problem hiding this comment.
Well, this probably should be done different way...
But what do you mean by "Bazel will remove the sandbox at the end of the run"?
Do you mean bazel clean?
There was a problem hiding this comment.
No, I may have said it wrong.
I meant that Bazel will run it in a clean state each time.
There was a problem hiding this comment.
My bad, I thought this was teardownClass.
| (self.project_dir / "MODULE.bazel").write_text( | ||
| MODULE_TEMPLATE.format(rules_path=self.rules_path) | ||
| ) | ||
| (self.project_dir / "WORKSPACE").touch() | ||
|
|
There was a problem hiding this comment.
We want to test Bazel 6 without the bzlmod system too!
There was a problem hiding this comment.
Right, Bazel 6 is not supported in this proto yet
| # FIXME: output 'analysis/codechecker_per_file/data/src-emit.cpp_clangsa.plist' was not created | ||
| # ":codechecker_per_file", | ||
| ], |
There was a problem hiding this comment.
This seems to only happen in mise.
This does not happen in any CI or when simply running it locally.
I do not agree with disabling this for the sake of mise passing.
There was a problem hiding this comment.
This sounds like a big bug to fix! In the test or in mise.
We MUST NOT accept "works for me" excuses.
There was a problem hiding this comment.
I disagree. That is likely a bug in mise that is irreproducible locally or in CI. I think its unreasonable to support it like this. We landed mise (#132 (review)) and micromamba (#167 (review)) on the condition that we will see how they can assist testing, but will not unconditionally fix or hack around related breakages until they are proven to be reliable.
Szelethus
left a comment
Szelethus
left a comment
There was a problem hiding this comment.
I like the approach! Would it be okay if, while preserving the main idea, we polished the PR up?
|
The approach is actually questionable - run bazel from bazel. Anyway, I have 2 conclusions in mind:
|
Why: The existing Python/Bash FOSS tests are hard to understand and extend, they are also very slow and cannot be integrated into Bazel environment. What: Add test/bazel/ with a foss_test() macro that generates sh_test() targets for FOSS projects. Test: bazel test //test/bazel/...
|
Sure! Lets think of a solution that is actionable and can be implemented incrementally. I don't mean this to come off the wrong way, but our bazel 8 support project showed how striving for perfection was the enemy of progress. Our foss tests so far have borne some fruit, even if they are imperfect. |
3 participants
Why:
The existing Python/Bash FOSS tests are hard to
understand and extend, they are also very slow
and cannot be integrated into Bazel environment.
What:
Add test/bazel/ with a foss_test() macro
that generates py_test() targets for FOSS projects.
Test:
bazel test //test/bazel/...