Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kernel: tests: Evaluate "platform_allow" usage in kernel tests. #51323

Closed
PerMac opened this issue Oct 17, 2022 · 4 comments
Closed

kernel: tests: Evaluate "platform_allow" usage in kernel tests. #51323

PerMac opened this issue Oct 17, 2022 · 4 comments
Labels
area: Tests Issues related to a particular existing or missing test Enhancement Changes/Updates/Additions to existing features

Comments

@PerMac
Copy link
Member

PerMac commented Oct 17, 2022

Problem
Platform_allow lists in kernel's testcase.yamls are incorrectly blocking testing scope.

Describe the solution you'd like
Every test scenario under tests/kernel which defines platform_allow has to be evaluated. Platform_allow lists should be replaced with other filtration means. To limit the CI execution scope one can use integration_platforms which will also enforce such test execution in the CI (skips are turned to errors in the CI for integration_platforms). Platform_allow lists should only be allowed for cases where there is no other means of filtration or tests are not portable (e.g. ones which require dedicated overlays for each platform).

Describe alternatives you've considered
I don't see other approach to fix the issue.

Additional context
IMO it is more a bug than an enhancement but due to the scope I put it under "enhancement". This problem is also not limited to kernel tests, but due to the scope I believe evaluation of platform_allow abuse can be limited to kernel tests in this task and we will continue solving the issue in following tasks.
Platform_allow is abused and used incorrectly. Looking at examples it seems it was often added just to limit the CI execution scope. Or because a developer only tested the solution on certain boards and didn't care about other platforms. Platform_allow is a hard filter preventing testing on anything outside of it. It also makes tests hard to maintain, since it needs constant updating with new boards being added (which is not happening in most cases...).
This evaluation is especially important in context of on-target testing and selecting "tier 1" boards. Kernel tests will be crucial for such boards. However, it seems there might be quite many artificial skips due to platform_allow misuse.
An example showing the problem: https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/kernel/gen_isr_table/testcase.yaml#L11
The platforms in platform_allow seem totally arbitrarily selected and blocks testing on vast majority of platforms.

@PerMac PerMac added the Enhancement Changes/Updates/Additions to existing features label Oct 17, 2022
@PerMac
Copy link
Member Author

PerMac commented Oct 17, 2022

@nashif @andyross I am not sure who to assign to this task. It requires kernel domain knowledge which I lack.

@henrikbrixandersen henrikbrixandersen added the area: Tests Issues related to a particular existing or missing test label Oct 18, 2022
@nashif
Copy link
Member

nashif commented Oct 27, 2022

@nashif @andyross I am not sure who to assign to this task. It requires kernel domain knowledge which I lack.

I do not think it requires any kernel domain knowledge, my suggestion is to dive into that and start making changes and raise specific issues that can't be resolved in a trivial way as issues. You probably can start by providing examples where you think domain knowledge is needed.

@nashif
Copy link
Member

nashif commented Oct 27, 2022

looking at tests/kernel and ignore the linker_generator tests, which use platform_allow in a valid way, I get:

events/event_api/testcase.yaml:    platform_allow: qemu_x86
fpu_sharing/float_disable/testcase.yaml:    platform_allow: qemu_x86 qemu_x86_lakemont ehl_crb acrn_ehl_crb
fpu_sharing/float_disable/testcase.yaml:    platform_allow: qemu_x86 qemu_x86_lakemont ehl_crb acrn_ehl_crb
fpu_sharing/generic/testcase.yaml:    platform_allow: qemu_x86 qemu_x86_lakemont
fpu_sharing/generic/testcase.yaml:    platform_allow: qemu_x86 qemu_x86_lakemont
gen_isr_table/testcase.yaml:    platform_allow: bbc_microbit atsamr21_xpro nrf51dk_nrf51422 nucleo_g071rb qemu_cortex_m0
gen_isr_table/testcase.yaml:    platform_allow: bbc_microbit atsamr21_xpro nrf51dk_nrf51422 nucleo_g071rb qemu_cortex_m0
mem_heap/shared_multi_heap/testcase.yaml:    platform_allow: qemu_cortex_a53 mps2_an521
mem_protect/demand_paging/testcase.yaml:    platform_allow: qemu_x86_tiny
mem_protect/mem_map/testcase.yaml:    platform_allow: qemu_x86_64
mem_protect/mem_map/testcase.yaml:    platform_allow: qemu_x86_64
mem_protect/mem_map/testcase.yaml:    platform_allow: qemu_x86_64
mem_protect/mem_protect/testcase.yaml:    platform_allow: efr32_radio_brd4180a mps2_an521 nrf9160dk_nrf9160
mem_protect/userspace/testcase.yaml:    platform_allow: efr32_radio_brd4180a mps2_an521 nrf9160dk_nrf9160
smp_boot_delay/testcase.yaml:    platform_allow: intel_adsp_cavs15 intel_adsp_cavs18 intel_adsp_cavs20 intel_adsp_cavs25
timer/cycle64/testcase.yaml:    platform_allow: native_posix
timer/timer_monotonic/testcase.yaml:     platform_allow: ehl_crb
workq/critical/testcase.yaml:    platform_allow: nsim_sem_mpu_stack_guard

So, it is not that bad actually and we should be able to deal with them individually...

@nashif
Copy link
Member

nashif commented Nov 19, 2022

kernel is not the only problem area, this issue is across the tree, some of the items above were already addressed, if there are any remaining issues, in kernel or outside the kernel, this should be a bug.

@nashif nashif closed this as not planned Won't fix, can't repro, duplicate, stale Nov 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Tests Issues related to a particular existing or missing test Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

No branches or pull requests

3 participants