Allow to filter testsuite roots #84225
Conversation
nordic-piks
force-pushed
the
allow_to_filter_testsuite_roots
branch
2 times, most recently
from
c1ef16f to
6700fd7
Compare
zephyrbot
requested review from
fabiobaltieri,
gchwier,
hakehuang,
KamilxPaszkiet,
kartben,
LukaszMrugala,
nashif and
stephanosio
nordic-piks
force-pushed
the
allow_to_filter_testsuite_roots
branch
from
6700fd7 to
ab20aa5
Compare
There was a problem hiding this comment.
Excluding tests by path is not as clean as you think, using tags, even if flawed in some areas is much more granular and accurate. The goal was to move to more granular and test identifier based filtering, which is more accurate. Initial step was done in the maintainer file:
Line 2502 in d1d85fa
First commit in this PR can be considered, but changes to test_plan IMO need some more thought and coordination.
what tests are not tagged? AFAIK tagging is almost everywhere, we have too much tagging if you ask me.
same applies for path based filtering, there is no easy way to match a code change to the tests related to this code. You are making an assumption that the test tree structure is complete and accurate but that is not the case, same issue with tagging. |
IMHO, test path filtering gives more freedom how to set scope and dependencies. To get more granularity you can write more specific paths and dependencies - which can be similar to the style of This is similar if you want to use tags in granular way. I wrote about freedom as if you for some reason want to have more general approach, you can do like this (nordic example): This is harder to do that with tags. Thank you for that
Do you have some PR or proposal? Just wanted to have a look how it is designed.
Regarding changes in test_plan, I intentionally made it optional, there is no test path filtering enable by default or in the zephyr CI. My idea was if someone want to use that kind of filtering, it will provide its own configuration file. As I look more on and files: Can Still I would vote for my PR so that rules can be adjusted in the downstream. |
Indeed, I was wrong regarding tags, sorry for that.
I am aware that with hardcoding dependencies I take a risk that they will be not accurate. But with benefit of having much faster CI I can accept that risk and act accordingly when regression happens.
This will create the whole output structure and at each test folder there will be file |
scripts/ci/test_plan.py
Outdated
| @@ -443,6 +464,9 @@ def parse_args(): | |||
| "the file need to correspond to the test scenarios names as in " | |||
| "corresponding tests .yaml files. These scenarios " | |||
| "will be skipped with quarantine as the reason.") | |||
| parser.add_argument('--testsuite-excludes-def', | |||
There was a problem hiding this comment.
nit: maybe consider changing this name to something more direct, e.g. --testsuite-excludes-file? it took me some time to figure out what this variable is responsible for from the code and only reading its description sorted it out.
There was a problem hiding this comment.
Changed, thanks :)
Allow to exclude specific paths from testsuite roots. It takes testsuite roots and filter out one which matches provided pattern, accepting Unix shell-style wildcards. Signed-off-by: Piotr Kosycarz <[email protected]>
Allow to filter out testsuites base on changed files. This is allow for similar way of excluding tests as for tags, however now there will be possible to specify paths for testsuites that should be excluded if certain files were not changed. Signed-off-by: Piotr Kosycarz <[email protected]>
nordic-piks
force-pushed
the
allow_to_filter_testsuite_roots
branch
from
ab20aa5 to
4612909
Compare
This PR contains 2 changes: for twister and test_plan (the one used at CI for test scope selection).
For twister, it adds a new parameter
--testsuite-exclude-pathwhich can be used to exclude the test suite path fromdiscovering tests (base on
--testsuite-root).So you can use
--testsuite-root tests --testsuite-exclude-path "*tests/kernel*" --testsuite-exclude-path "*tests/lib*"to run all tests except those in the tests/kernel and tests/lib paths.This can be useful to limit the number of executed tests (e.g. if it is known that nothing related to these tests has changed).
For test_plan, an option is added to include the relationship between test suite paths and modified files in the test scope selection process. The new way works almost the same as filtering by tags, but it filters by test suite paths. The relationship to the modified files is the same i.e. if the PR does not modify the specified files/paths, the test suite path is removed from execution.
Test suite path based filtering has several advantages:
The downside may be that if you add a new test and its path is already listed in excludes, it will not be automatically executed. The opposite is true for tags, if the new test does not have one, it will not be filtered out.
Tag-based filtering and test suite path filtering can be used together or separately.
The
scripts/ci/testsuite_excludes.yamlhas example content (but valid), I plan to extend it for all tests and samples (not at this PR). If wanted, I can share that dependency list so it can be used for zephyr CI.This PR does not affect test scope selection used currently at zephyr CI! (new filtering is not enabled there).