Closed dcermak closed 5 months ago
LGTM. I'd just do the determination of affected tests by looking at the list of modified files but this works as well.
I don't see this working as expected. It is unlikely that external users will know what tests to run. Plus, you need to remember to edit the PR text before submitting it.
It should be the other way around perhaps. Run a few tests by default always, and an "admin" comments something to enable the rest of the tests to run.
Another issue is that all env tests are required, if you run a subset only, the PR gets "blocked" by default.
I don't see this working as expected. It is unlikely that external users will know what tests to run.
We get zero external contributors. The majority of the PRs are by the BCI or the QA team who will know what to run.
Plus, you need to remember to edit the PR text before submitting it.
That's why there's a template.
It should be the other way around perhaps. Run a few tests by default always, and an "admin" comments something to enable the rest of the tests to run.
I did that in #461 and Dirk objected. Plus this solution is much simpler.
Another issue is that all env tests are required, if you run a subset only, the PR gets "blocked" by default.
Yes, but an admin can override that. Also, we can relax the required CI jobs.
Alexandre Vicenzi @.***> writes:
@alexandrevicenzi commented on this pull request.
@@ -0,0 +1,8 @@ +<!-- +Thanks for sending a pull request! + +In case you are changing only tests for a specific environment and don't need to +run all test environments, add the following line to your PR description on a +separate line! [CI:TOXENVS] foo,bar +
I noticed that in some of my PRs, GitHub interface grabs the last commit message and it would possibly hide this template.
Will this template show all the time? or can GitHub hide it in some scenarios?
Github can hide it sometimes, iirc if you create a PR explicitly from a branch and only have one commit there.
@dirkmueller @alexandrevicenzi Any opinions how to move forward here?
@dirkmueller @alexandrevicenzi Any opinions how to move forward here?
see my earlier message already at https://github.com/SUSE/BCI-tests/pull/464#issuecomment-2084952391
Feel free to merge it.
[CI:TOXENVS] base,minimal