Define how to run tests#218
Conversation
|
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". |
3349517 to
39cd357
Compare
nettle
left a comment
There was a problem hiding this comment.
This looks to me as introduction of venv + llvm environment :)
It should go under .ci/ as yet another dev env along with mise and micromamba.
And it should be a separate review, not a part of "Define how to run tests".
|
Fair! That raises some questions about whether the file |
Actually I think venv or uv could be useful :)
To be honest just adding such script does not bring much value without defining the environment I think. |
This is actually interesting!
Well, setting up the environment "manually" sounds extremely unreliable. |
|
I understand that this patch is now beyond a simple "single point of entry" test script. Lets take a step back, I fear some misunderstandings have to be beaten into my head. The end goal (browsing through our last meeting notes) is the following. We should consider a PR ready to merge (after reviewer approval(s)) if:
And of course it would be great if those CI environments could be replicated and run locally with relative ease, with mise/micromamba. What can we do today that isn't a hack and gets us further towards this end goal? I suggest that
Let's land and finish up the few, small steps we agreed upon, so we don't have so many threads open. |
It was never been "a simple single point of entry test script" to me :)
Exactly! That's the goal of this task - how to setup the environment? what tests to run? how?
Yes, that we should do. However, #219 is not a showstopper for this task.
No, this does not help much without setting up reliable environment.
I'm sure this is the main thing we need to do here.
I would suggest addressing the main problem - reliable environment. |
|
We chatted with @furtib a bunch, and I think it'd be best to pursue micromamba! We'll come back with some more concrete ideas once time allows. |
|
|
||
| # Clean up previous environment | ||
| info "Cleaning up previous environment [$ENV_NAME]..." | ||
| chmod -R +w $THIS_DIR/micromamba/envs/dev |
There was a problem hiding this comment.
I think dev is actually $ENV_NAME
| @@ -0,0 +1,13 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
What I think is that run_tests.sh is actually micromamba dependent, so maybe we should keep it as .ci/micromamba/run_tests.sh ?
| chmod -R +w $THIS_DIR/micromamba/envs/dev | ||
| rm -rf $THIS_DIR/micromamba/envs/dev |
There was a problem hiding this comment.
Hm... Are you sure this is safe? There reason might be running session. Maybe proper deactivation may help?
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?