Zephyr in tree samples CI coverage criteria and elastic quality maintain policy enhancement

[hakehuang]

Introduction

Samples in Zephyr tree have been a very critical code reference for Zephyr RTOS. the basic sample concepts are defined here:
[samples definition
Goal of samples need to be more clear as to:

1. Samples are constructed in Zephyr RTOS and its eco-systems, so its has to be comply with the zephyr-eco system, so we expect Sample is portable in Zephyr-ecosystem, constrained by Zephyr RTOS's meta-data/device tree settings/Kconfig or other filterable tags maintained in tree.
2. The samples quality need to be maintained, the scope can be enhanced gradually, but the minimal sets shall be buildable in defined platforms, and runs by board testing CI on those platforms(not necessary full function check, but ensure there is no issues for the application to start).
3. long term goal for samples is to have full function validation by common tooling(defined by fixtures and harness by now).
4. the samples portability shall be considered, and CI shall help to enhance the portability.
5. portable and function issue is a valid issue that community need take a look, the owner shall be (sample owner)/(platform owner)/(dedicated working group)/(test working group)
6. the sample user case shall be well documented for at least one platform.

Problem description

1. Some samples are not CI(twister) runnable, it will be either filter out by harness or just timeout.
2. Abuse the platform_allow without any clear reason on this.
3. Sample ownership is not clear.
4. CI for samples need consider upstream simplicity and downstream portability.
5. Rules by now are:

The bottom line here is:

• Samples SHALL not be used or treated as tests
• The tests in the context of the sample verifies the sample as documented and with supported platforms that have been verified to work with the sample, i.e. by whoever wrote the sample.
• There is no expectation that a sample shall build and run on platforms that are not documented or verified
• Filters shall be used to limit the test to the platforms that are known to run the sample as per the documentation and expectation of the sample code. This includes ANY filters, i.e. platform_allow is FULLY valid if a sample is known to only work on a limited set of platform and was VERIFIED on those.
• A filter might be wider in scope and might pull in platforms that were never verified or tested to work with the sample, or the opposite, a filter might be too limited and potential platforms that would be able to run the sample will be excluded. This is a problem we have right now and what this discussion is about. We can fix this in different ways, but the above points and documented policy SHALL be observed.

• Samples are not Tests
• There is no expectation from samples to build and run on ALL platforms
• Platform X failing on sample when this platform was never tested, verified and documented is not a BUG
• If someone thinks a certain platform should be used with a specific sample, they should be able to modify the sample code to support that platform, document this as a supported platform and verify it actually works.

Concerns with above bottom line are

• Sample writer needs to be the maintainer, and if it is not runnable in CI, the quality is merely depends on the maintainer.
• Portability of sample is not considered, which make samples in tree less attraction. Porting sample to a new platform shall always use dts/kconfig/tags to extend( and change source code by based on those), not by hardcode for different platforms, and this needs considered in sample design.
• filter by tag/Kconfig/DTS is reasonable, but platform_allow is filtered by no reason. Some reasons are:

-  there are many samples with such platform_allow.
-  No one, even the owner does not know whether it can be build / run on other platform

4. leave samples in free-style to evolve would make the Zephyr sample code quality un-consistent, that shall be avoid.

Proposed change

1. Stating that we are less prefer to platform_allow, instead that dts/Kconfig/feature tag is recommend.
2. Enhance sample maintainer group(e.g system group), and assign maintainer to each.
3. add integration_platforms to sample is a MUST, and CI in upstream will only build for those platforms. And board testing from upstream can focus on integration_platforms. integration_platform shall not keep adding if there is no significant differences.
4. issues with filtered valid platform on given samples are valid issues(enhancement issues).
5. establish a design guide and porting guide for sample in general.
6. downstream testing for filtered platform need maintained by each vender.

Detailed RFC

• document that platform_allow is less acceptable, using dts/Kconfig/tag is the recommend way by now, and open to future change.
• allocate samples to dedicated working group.
• consider to have architecture working group to maintain the sample in general.
• tag as portability for issues found in the future as enhancement issue.
• enhance the existing sample code, to allow twister CI can check at least run without initial/system fault.
• open to community to allow enhancement to change platform_allow by dts/Kconfig/tag, test working group and other function group can propose.

[ ] net
[ ] bluetooth
[ ] boards
[ ] modules
[ ] sensor
[ ] subsys

        [ ] bindesc
        [ ] canbus
        [ ] console
        [ ] dap
        [ ] debug
        [ ] demand_paging
        [ ] display
        [ ] edac
        [ ] fs
        [ ] input
        [ ] ipc
        [ ] llext
        [ ] logging
        [ ] lorawan
        [ ] mgmt
        [ ] modbus
        [ ] nvs
        [ ] pm
        [ ] portability
        [ ] profiling
        [ ] rtio
        [ ] sensing
        [ ] settings
        [ ] shell
        [ ] sip_svc
        [ ] smf
        [ ] subsys.rst
        [ ] task_wdt
        [ ] testsuite
        [ ] tracing
        [ ] usb
        [ ] usb_c
        [ ] zbus

[ ] drivers

       [ ] adc
       [ ] audio
       [ ] auxdisplay
       [ ] can
       [ ] charger
       [ ] clock_control_litex
       [ ] clock_control_xec
       [ ] counter
       [ ] crypto
       [ ] dac
       [ ] display
       [ ] drivers.rst
       [ ] eeprom
       [ ] espi
       [ ] ethernet
       [ ] flash_shell
       [ ] fpga
       [ ] gnss
       [ ] haptics
       [ ] ht16k33
       [ ] i2c
       [ ] i2s
       [ ] ipm
       [ ] jesd216
       [ ] kscan
       [ ] lcd_cyclonev_socdk
       [ ] lcd_hd44780
       [ ] led
       [ ] led_strip
       [ ] lora
       [ ] mbox
       [ ] mbox_data
       [ ] memc
       [ ] misc
       [ ] mspi
       [ ] peci
       [ ] ps2
       [ ] rtc
       [ ] smbus
       [ ] soc_flash_nand
       [ ] soc_flash_nrf
       [ ] spi_bitbang
       [ ] spi_flash
       [ ] spi_flash_at45
       [ ] spi_fujitsu_fram
       [ ] uart
       [ ] video
       [ ] virtualization
       [ ] w1
       [ ] watchdog

Proposed change (Detailed)

proposed PRs for such as below:

#84458
#85446

Dependencies

not clear

Concerns and Unresolved Questions

Need feedback

Alternatives

we can keep Sample as it is now, but the defects are lists above as concerns.

[golowanow]

change CI for samples to build / test only for integration_platforms.

As I understand the problem is in different focus on samples at community CI and from vendor's CI perspective on how much resources to assign on samples' verification and who takes care of issues found there.

I think one way to manage the scope for samples at the project's CI is to list the eligible samples as an explicit test plan in dedicated yaml config files (with maintainers assigned) using levels (twister --level samples --test-config <yaml_file> --testsuite-root ./samples), so only these maintained samples will be in the scope of build check and run on simulators at community.
Each vendor or maintainer can develop additional plans for samples, which are targeted to some area of interest either as a suggested scope for the community, or as their area of verification.

[nashif]

• Samples are constructed in Zephyr RTOS and its eco-systems, so its has to be comply with the zephyr-eco system, so we expect Sample is portable in Zephyr-ecosystem, constrained by Zephyr RTOS's meta-data/device tree settings/Kconfig or other filterable tags maintained in tree.

per current definition:

A sample is a concise Zephyr application that provides an accessible overview of one or more features, subsystems, or modules. This includes kernel services, drivers, protocols, etc. Samples should be limited in scope and should focus only on demonstrating non-trivial or essential aspects of the subsystem(s) or module(s) being highlighted.

https://docs.zephyrproject.org/latest/samples/sample_definition_and_criteria.html#definition-and-criteria already talks about portability and being generic.

Samples are provided to demonstrate functionality of a subsystem, it is usually provided alongside a new feature and provided by the same contributor. A sample author is expected to document and test functionality of the sample and provide information about hardware the sample can run on. A sample author is not expected to port and verify the sample on all platforms supported by Zephyr. Ideally we want samples to be written and used by available platforms and where applicable, run on simulators.

Samples serve as reference for application writers and users of Zephyr, they are not neither used to verify functionality and correctness of features in Zephyr, nor for platform verification.

CI is used to verify that the sample as provided and as documented and on the supported hardware, as provided by the author. Most samples are verified for buildability in CI, CI does not verify if a sample works as documented. Some samples however can be verified in CI based on the output they provide on the console and get full coverage in CI using simulation platforms.

• The samples quality need to be maintained, the scope can be enhanced gradually, but the minimal sets shall be buildable in defined platforms, and runs by board testing CI on those platforms(not necessary full function check, but ensure there is no issues for the application to start).

sure, see above.

• long term goal for samples is to have full function validation by common tooling(defined by fixtures and harness by now).

Nice goal, but before getting "full function validation" on samples we better focus on "full function validation" of Zephyr features and invest in functional testing in general, samples might benefit from that, but also adds lots of constraints on how you right samples and gets us in the realm of application testing, which is at a completely different level. We have this in some areas (Bluetooth maybe), but how do you test samples demonstrating displays and lvgl for example?

• the samples portability shall be considered, and CI shall help to enhance the portability.

Portability is already documented and required, but is not a mandatory right now. There are different levels of portability. If your understanding of portability is that it has to work and build on each platform in Zephyr, then we are basically increasing the entry barrier for writing samples and nobody will want to write samples for ~1000 platforms.

• portable and function issue is a valid issue that community need take a look, the owner shall be (sample owner)/(platform owner)/(dedicated working group)/(test working group)

Depends on your definition of portability. If a sample is provided for platform A and documented to run on A and does not build on platform B then it is not a bug. Someone with platform B can always come in and enahance it to run on that platform.

• the sample user case shall be well documented for at least one platform.

sure.

3. Sample ownership is not clear.

zephyr/MAINTAINERS.yml

Line 3281 in 95179b2

Samples:

2. Abuse the platform_allow without any clear reason on this.

https://docs.zephyrproject.org/latest/samples/sample_definition_and_criteria.html#definition-and-criteria:~:text=Use%20platform_allow%20to%20limit%20test%20to%20the%20platforms%20the%20sample%20was%20actually%20verified%20on.%20Those%20platforms%20should%20be%20listed%20in%20the%20sample%E2%80%99s%20README%20file.

5. Rules by now are:

guidelines are actually here: https://docs.zephyrproject.org/latest/samples/sample_definition_and_criteria.html#definition-and-criteria

1. Stating that we are phasing out platform_allow.

Why? A sample or a test that can get away without using plaform_allow is fine, platform_allow is just for the test (in the context of the sample) it does not mean this sample can't run on other platforms. What targets are supported with a specific sample should be part of the docs of the sample.

Concerns with above bottom line are

• Sample writer needs to be the maintainer, and if it is not runnable in CI, the quality is merely depends on the maintainer.

We have maintainer for samples in general, to make sure samples are consistent and provided in a way that helps users and developers.
Then we have the owner/maintainer of the code, usually the maintainer of the subsystem.

This is similar to anything else in Zephyr.

• we have subsystem maintainers, i.e. I2C
• then we have maintainers of platforms and driver instances, those who add a specific i2c driver for a specific platforms. This is not different.

• Portability of sample is not considered, which make samples in tree less attraction. Porting sample to a new platform shall always use dts/kconfig/tags to extend( and change source code by based on those), not by hardcode for different platforms, and this needs considered in sample design.

Writing samples in a generic way and considering portability is already described here: https://docs.zephyrproject.org/latest/samples/sample_definition_and_criteria.html#definition-and-criteria

No need to repeat that.

• filter by tag/Kconfig/DTS is reasonable, but platform_allow is filtered by no reason. Some reasons are:

• there are many samples with such platform_allow.
• No one, even the owner does not know whether it can be build / run on other platform

???

4. leave samples in free-style to evolve would make the Zephyr sample code quality un-consistent, that shall be avoid.

not sure what free-style means here and why you think it is free-style now?

Proposed change

1. Stating that we are phasing out platform_allow.

This is not a solution, this is just a recipe for more problems. platform_allow can be replaced or removed only where it make sense, try removing platform_allow from samples under samples/bluetooth. So unless you can provide a better way, we are stuck with this.

2. establish sample maintainer group(e.g system group), and assign maintainer to each.

We have a sample are in zephyr and we have a maintainer, there is no need to create more groups.

3. add integration_platforms to sample is a MUST, and CI in upstream will only build for those. platforms. And board testing from upstream can focus on integration_platforms. integration_platform shall not keep adding if there is no significant differences.
4. issues with filtered valid platform on given samples are valid BUGs.

yes, thats why we use platform_allow (which is also a filter), and if there is an issue with those, it is a bug.

5. establish a design guide and porting guide for sample in general.

if you think we do not have one already, propose one.

6. downstream testing for filtered platform need maintained by each vender.

[hakehuang]

@nashif , thanks a lot for feedbacks, let me clarify my thought below.

Samples are provided to demonstrate functionality of a subsystem, it is usually provided alongside a new feature and provided by the same contributor. A sample author is expected to document and test functionality of the sample and provide information about hardware the sample can run on. A sample author is not expected to port and verify the sample on all platforms supported by Zephyr. Ideally we want samples to be written and used by available platforms and where applicable, run on simulators.

I agree. But the simulators can not cover most of the necessary features, as many hardware are missing, And we do not see strong roadmap from any simulator's roadmap.
What I am proposing is that it is not the original contributor's responsibility to make his sample work on every platform, but need specify on the actual dependency by Kconfig/dts, not by platform_allow in general. And this is the responsibilities of the sample maintainer to response on its porting issues. To achieve this, sample owner needs a easy way to run the application, ideally, just like other applications with twister.

Samples serve as reference for application writers and users of Zephyr, they are not neither used to verify functionality and correctness of features in Zephyr, nor for platform verification.

it is not, I agree. My proposal is more from the ego system point of view. I find several issues which related Kconfg/dts dependency with samples/basic/minimal the Kconfig dependency change can easily break the build. Which needs attention, as Zephyr-RTOS states that it is highly configurable.

Nice goal, but before getting "full function validation" on samples we better focus on "full function validation" of Zephyr features and invest in functional testing in general, samples might benefit from that, but also adds lots of constraints on how you right samples and gets us in the realm of application testing, which is at a completely different level. We have this in some areas (Bluetooth maybe), but how do you test samples demonstrating displays and lvgl for example?

Moving Samples into test is a good idea, I can add to this proposal, which there is a dependency on what the constrains are, which is better coming from the original sample designer.

Portability is already documented and required, but is not a mandatory right now. There are different levels of portability. If your understanding of portability is that it has to work and build on each platform in Zephyr, then we are basically increasing the entry barrier for writing samples and nobody will want to write samples for ~1000 platforms.

The idea is that in upstream we only cares on integration_platform(by now it is called platflom_allow), the differences is that platform_allow will excluded by twister, and I want change that. As I mention above, there is no needs for original owner to maintain 100 platforms, the sample maintainer needs take care and assign issue to the platform owner, so it will not blocking the upstream releases. It helps vender to identify issues in their features, and if the vender does not care, no one cares, but the issue is identified already. In this way, a well maintained platforms will quick qualified by twister testing.

Depends on your definition of portability. If a sample is provided for platform A and documented to run on A and does not build on platform B then it is not a bug. Someone with platform B can always come in and enahance it to run on that platform.

if it is not a bug. The how could we know it is broken? or we can mark it as feature enhancement issue?

https://docs.zephyrproject.org/latest/samples/sample_definition_and_criteria.html#definition-and-criteria:~:text=Use%20platform_allow%20to%20limit%20test%20to%20the%20platforms%20the%20sample%20was%20actually%20verified%20on.%20Those%20platforms%20should%20be%20listed%20in%20the%20sample%E2%80%99s%20README%20file.

There is only 2 maintainer for all samples, which is not possible. ideally, each sample will have a maintainer who are from the related working group.

Why? A sample or a test that can get away without using plaform_allow is fine, platform_allow is just for the test (in the context of the sample) it does not mean this sample can't run on other platforms. What targets are supported with a specific sample should be part of the docs of the sample.

What I am propose is to accommodate the sample in twister CI, so that is it easy for maintainer. platform_allow or integration_platform is just a name. The idea is that downstream CI with twister can help to catch issues without change, and maybe enhance issue will be reported from each vender. Also, any developer can find this issue with CI, and provide fixes with twister's CI this could be much more easier.

This is similar to anything else in Zephyr.

• we have subsystem maintainers, i.e. I2C
• then we have maintainers of platforms and driver instances, those who add a specific i2c driver for a specific platforms. This is not different.

This could be different, Samples are always include several modules/ firmware, e.g. we have sensor working group, who need to know the peripheral driver, and the sensor fw/algorithm. but sensor team may only care sensor, what about motor control? and all others?

Writing samples in a generic way and considering portability is already described here: https://docs.zephyrproject.org/latest/samples/sample_definition_and_criteria.html#definition-and-criteria

No need to repeat that.

I just want to add 1 requirement for constrains, i.e, what is the constrains on particular sample.

No one, even the owner does not know whether it can be build / run on other platform
???

As we only test with platform_allow, so we will not see porting issue. And people try to port this will have to resolve issues by themselves, and submit PR. this proposal is just to make this process easier.

not sure what free-style means here and why you think it is free-style now?

Sorry, forgive my poor language, may be limited supported status would be better.

This is not a solution, this is just a recipe for more problems. platform_allow can be replaced or removed only where it make sense, try removing platform_allow from samples under samples/bluetooth. So unless you can provide a better way, we are stuck with this.

I agree. maybe we can state that platform_allow is not recommend, and try to use dts/Kconfig first.

We have a sample are in zephyr and we have a maintainer, there is no need to create more groups.

I suppose the sample owner is too few, suggest to break-down the samples and adding more specific maintainer.

if you think we do not have one already, propose one.

as above, just add one statement on constrains for porting.

[nashif]

Samples are provided to demonstrate functionality of a subsystem, it is usually provided alongside a new feature and provided by the same contributor. A sample author is expected to document and test functionality of the sample and provide information about hardware the sample can run on. A sample author is not expected to port and verify the sample on all platforms supported by Zephyr. Ideally we want samples to be written and used by available platforms and where applicable, run on simulators.

I agree. But the simulators can not cover most of the necessary features, as many hardware are missing, And we do not see strong roadmap from any simulator's roadmap. What I am proposing is that it is not the original contributor's responsibility to make his sample work on every platform, but need specify on the actual dependency by Kconfig/dts, not by platform_allow in general. And this is the responsibilities of the sample maintainer to response on its porting issues. To achieve this, sample owner needs a easy way to run the application, ideally, just like other applications with twister.

I said, "where applicable".

Samples serve as reference for application writers and users of Zephyr, they are not neither used to verify functionality and correctness of features in Zephyr, nor for platform verification.

it is not, I agree. My proposal is more from the ego system point of view. I find several issues which related Kconfg/dts dependency with samples/basic/minimal the Kconfig dependency change can easily break the build. Which needs attention, as Zephyr-RTOS states that it is highly configurable.

The thing is, such samples are not there to be run on platforms, they actually are designed to build only and to exercise and show how to optimize for footprint and it is done with a selected set of platforms for demonstration purposes only. There is no requirement or expectation that any platform should be usable or configurable to operate for example without multithreading. When you remove filters and samples start failing, that is not an automatic bug.

Nice goal, but before getting "full function validation" on samples we better focus on "full function validation" of Zephyr features and invest in functional testing in general, samples might benefit from that, but also adds lots of constraints on how you right samples and gets us in the realm of application testing, which is at a completely different level. We have this in some areas (Bluetooth maybe), but how do you test samples demonstrating displays and lvgl for example?

Moving Samples into test is a good idea, I can add to this proposal, which there is a dependency on what the constrains are, which is better coming from the original sample designer.

I am not suggesting we should move samples to be tests. I am saying the investment in testing how a sample for subsystem X works should be better done in the test framework in general to cover unit tests and functional tests of the feature, samples can benefit from that down the line, but our goal should not be how to validate complex samples from the get go.

Portability is already documented and required, but is not a mandatory right now. There are different levels of portability. If your understanding of portability is that it has to work and build on each platform in Zephyr, then we are basically increasing the entry barrier for writing samples and nobody will want to write samples for ~1000 platforms.

The idea is that in upstream we only cares on integration_platform(by now it is called platflom_allow), the differences is that platform_allow will excluded by twister, and I want change that.

You can only change that by following the sample guidelines, if you want to remove platform_allow, you should apply another filter that is more flexible, just removing platform_allow is going to be a problem.

As I mention above, there is no needs for original owner to maintain 100 platforms, the sample maintainer needs take care and assign issue to the platform owner, so it will not blocking the upstream releases. It helps vender to identify issues in their features, and if the vender does not care, no one cares, but the issue is identified already. In this way, a well maintained platforms will quick qualified by twister testing.

As I said a above, samples can always be exteded in features to support new additional platforms, this should be done by the platform owner, not the sample owner. If you platform does not work with a sample, it is not a bug, it is a bug if the sample documents supporting a platform A that does not work.

docs.zephyrproject.org/latest/samples/sample_definition_and_criteria.html#definition-and-criteria:~:text=Use platform_allow to limit test to the platforms the sample was actually verified on. Those platforms should be listed in the sample’s README file.

There is only 2 maintainer for all samples, which is not possible. ideally, each sample will have a maintainer who are from the related working group.

Those are the maintainers for the samples as a subsystem or area, i.e. how we structure samples, how we write them, what the expectation and how do we make sure our samples are useful, each subsystem or area of the project owns their own specific samples, just search for samples in the MAINTAINER file and you will see tha many samples belong to the respective ares they are demonstrating.

[hakehuang]

The thing is, such samples are not there to be run on platforms, they actually are designed to build only and to exercise and show how to optimize for footprint and it is done with a selected set of platforms for demonstration purposes only. There is no requirement or expectation that any platform should be usable or configurable to operate for example without multithreading. When you remove filters and samples start failing, that is not an automatic bug.

I agree that the purpose if the sample is not to run on platforms or even build on possible platform. but as it is in basic folder, should we think about why not it can't be? maybe this is not a bug, but a feature enhancement.

I am not suggesting we should move samples to be tests. I am saying the investment in testing how a sample for subsystem X works should be better done in the test framework in general to cover unit tests and functional tests of the feature, samples can benefit from that down the line, but our goal should not be how to validate complex samples from the get go.

I see. and agree.

You can only change that by following the sample guidelines, if you want to remove platform_allow, you should apply another filter that is more flexible, just removing platform_allow is going to be a problem.

OK, I see, maybe should put platform_allow as less preferred.

As I said a above, samples can always be exteded in features to support new additional platforms, this should be done by the platform owner, not the sample owner. If you platform does not work with a sample, it is not a bug, it is a bug if the sample documents supporting a platform A that does not work.

the things is I want twister CI can catch such issue without addition efforts. this helps a lot for our downstream alignment with upstream.

[dleach02]

There is a point in @hakehuang RFC that is important and that is the use of "platform_allow". I personally believe this filter keyword should be removed. The filters associated with the sample/test yamls should stick to only DTS/Kconfig type capabilities of the platform as the indicator of whether this application should be able to compile/run on the target.

@nashif you make a valid argument that not all samples/tests are designed to be able to build or run on arbitrary targets but there seems to be a cheat going on to use platform_allow as a method to filter. When I see the list of failures that Hake identified with the basic\minimal sample I wonder what dts/configuration is missing and why that isn't part of the filter to this sample?

I've seen other samples/tests where we end up with this hugely growing list of allowed platforms which just doesn't scale very well.

[hakehuang]

@nashif I update the Proposal content based on what we have discussed, see the Proposed change. also I propose a draft PR
to demo this idea, #85446
now the platform_allow is changed to less preferred.