ci: add_test: using maintainer.yml to add testcases #42662
Conversation
hakehuang
requested review from
mbolivar-nordic,
galak and
nashif
as code owners
There was a problem hiding this comment.
How did you test it? I would like to see some demo. I have troubles understanding what your code is doing. It starts with descriptive names, but down the line it ends with
if [ e for e in __f if e in target_files ]:. It would be also easier to review if you'd add some comments explaining what the code is trying to achieve. E.g. I think I spent quite some time (too much) trying to figure out what L47-72 is doing
| for _tp in test_apps_path: | ||
| _apps_path = os.path.join(zephyr_base_dir, _tp) | ||
| for path in Path(_apps_path).rglob("testcase.yaml"): | ||
| _tp = os.path.relpath(path, zephyr_base_dir) | ||
| test_path_list.append(os.path.dirname(_tp)) | ||
| for path in Path(_apps_path).rglob("sample.yaml"): | ||
| _tp = os.path.relpath(path, zephyr_base_dir) | ||
| test_path_list.append(os.path.dirname(_tp)) | ||
|
|
There was a problem hiding this comment.
Can you refactor this part, so the same code is not repeated?
scripts/ci/test_plan.py
Outdated
| if '*' in item: | ||
| _files = Path(zephyr_base_dir).rglob(item) | ||
| _files = list(set(_files)) | ||
| __f = [os.path.basename(i) for i in _files] | ||
| else: | ||
| _files = Path(os.path.join(zephyr_base_dir, item)).rglob("*") | ||
| _files = list(set(_files)) | ||
| __f = [os.path.basename(i) for i in _files] |
There was a problem hiding this comment.
Can you refactor this part, to reduce repetitiveness?
scripts/ci/test_plan.py
Outdated
| if re.match('^tests', item): | ||
| _test_apps_path.append(item) | ||
| continue | ||
| if re.match('^samples', item): | ||
| _test_apps_path.append(item) | ||
| continue |
There was a problem hiding this comment.
How did you test it? I would like to see some demo. I have troubles understanding what your code is doing. It starts with descriptive names, but down the line it ends with
if [ e for e in __f if e in target_files ]:. It would be also easier to review if you'd add some comments explaining what the code is trying to achieve. E.g. I think I spent quite some time (too much) trying to figure out what L47-72 is doing
ok, I will add some comments to exaplin the purpose, and I write a standalone code to test this function before I put the code into ci.
based on modified .c/.h file, check maintainer.yaml for related sample/test application e.g. if modified file is dma_mcux_edma.c three test cases will be added ['tests\\drivers\\dma\\chan_blen_transfer', 'tests\\drivers\\dma\\chan_link_transfer', 'tests\\drivers\\dma\\loop_transfer'] Signed-off-by: Hake Huang <[email protected]>
|
I tried this, but can't see how you get the tests just based on the driver being changed. Also, how would this work for any driver? What are the expectations from the entries in the MAINTAINERS file to be able to support this in the most efficient way? Also see discussion here for something that is supposed to solve this problem: #38725 (comment) |
and all test applications inside the test.drivers.dma will be added to test plan. |
based on modified .c/.h file, check maintainer.yaml for
related sample/test application
e.g.
if modified file is dma_mcux_edma.c
three test cases will be added
['tests\drivers\dma\chan_blen_transfer',
'tests\drivers\dma\chan_link_transfer',
'tests\drivers\dma\loop_transfer']
Signed-off-by: Hake Huang [email protected]