twister: Add support for required snippets #61111

nordicjm · 2023-08-03T10:29:48Z

Adds support for twister to require using snippets on tests

nordicjm · 2023-08-03T10:30:54Z

@JordanYates Is this sort of what you were asking for? I don't see much use for it with the cdc-acm-console snippet given that it raises a warning for each invocation and twister instantly fails on that

nordicjm · 2023-08-15T07:46:48Z

@JordanYates ping

JordanYates

Generally it looks good. What do you mean by failing instantly on cdc-acm-console?
We would probably also need an additional twister arg to add snippet roots as well.

scripts/pylib/twister/twisterlib/testplan.py

nordicjm · 2023-08-15T08:36:57Z

Generally it looks good. What do you mean by failing instantly on cdc-acm-console?

It emits a build warning when USB CDC is enabled and logging is also enabled from https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/usb/device/class/cdc_acm.c#L63 and this will fail by default in twister

doc/releases/release-notes-3.5.rst

nashif · 2023-08-25T10:15:26Z

doc/releases/release-notes-3.5.rst

@@ -160,6 +160,10 @@ Build system and infrastructure

  * Added support for CodeChecker

+* Twister now supports ``required_snippets`` in testsuite .yml files, this can
+  be used to include a snippet when a test is ran (and exclude any boards from


... when a test is run

nashif

Do we have someone using this feature in the tree? Would be great to have some tests using this.

nordicjm · 2023-08-25T10:43:33Z

Do we have someone using this feature in the tree? Would be great to have some tests using this.

Not in tree other than the twister test cases no, the problem is that the only snippet in tree is the USB CDC console one which emits a build warning for a lot of boards because it reduces the USB log level, and when ran in twister that fails the build immediately

PerMac

A minor request for logging level. The other comment is a general question, as I'd like to understand the logic there. When I added cdc-acm-console to hello_world sample it seemed to work as I got extra OUTPUT: uart connected to pseudotty: /dev/pts/4 in the output.

scripts/pylib/twister/twisterlib/testplan.py

PerMac · 2023-08-25T14:12:49Z

scripts/pylib/twister/twisterlib/testplan.py

+                        if len(found_snippets[this_snippet].appends) > 0:
+                            continue


Can you explain these lines? I think I don't follow. While debugging I see that cdc-acm-console has len(found_snippets[this_snippet].appends) = 2 hence continuing, i.e. ignoring the rest of the logic, where it is decided if a matching board was found

This checks if it is a global snippet i.e. applies to all boards, or if it only applies to some boards, see https://docs.zephyrproject.org/latest/build/snippets/writing.html#by-name for an example

mbolivar-ampere

Looks like a good feature and no objections to the idea. One minor naming suggestion, nonblocking.

mbolivar-ampere · 2023-08-28T17:44:06Z

scripts/snippets.py

@@ -238,6 +238,22 @@ def process_snippets(args: argparse.Namespace) -> Snippets:

    return snippets

+def process_snippets_inter(requested_snippets, snippet_roots) -> Snippets:


I think this would be better named find_snippets_in_roots or something like that. I had a hard time guessing what process_snippets_inter meant when I first saw it earlier in the patch.

nashif · 2023-08-28T17:49:42Z

Not in tree other than the twister test cases no, the problem is that the only snippet in tree is the USB CDC console one which emits a build warning for a lot of boards because it reduces the USB log level, and when ran in twister that fails the build immediately

ok, who is using this feature then and how? I am trying to understand the usecase here and this and it would help if we have a user in-tree beside the unit test.

nordicjm · 2023-08-29T06:30:19Z

Not in tree other than the twister test cases no, the problem is that the only snippet in tree is the USB CDC console one which emits a build warning for a lot of boards because it reduces the USB log level, and when ran in twister that fails the build immediately

ok, who is using this feature then and how? I am trying to understand the usecase here and this and it would help if we have a user in-tree beside the unit test.

It was a feature requested by @JordanYates from the original implementation PR. Currently there cannot be an in-tree usage of this because there is only one in tree snippet, but as time passes and more snippets are added, then it would be a good fit for having usage by tests/samples in tree.

PerMac · 2023-08-29T11:24:30Z

I realized you forgot to add "required_snippets" to the schema for a common section. Adding this entry in common causes

ERROR   - /home/maciej/zephyrproject/zephyr/samples/hello_world: can't load (skipping): SchemaError(msg='Schema validation failed:
 - Key 'required_snippets' was not defined. Path: '/sample'.')

nashif · 2023-08-29T12:05:20Z

Currently there cannot be an in-tree usage of this because there is only one in tree snippet, but as time passes and more snippets are added, then it would be a good fit for having usage by tests/samples in tree.

maybe it is time to add more snippets, software only, i.e. something that can be used as an example without dependency on hardware, I just added

commit 3f3374bbb96a2a84984905e7f934385400801bd4 (HEAD -> twistersnippets)
Author: Anas Nashif <anas.nashif@intel.com>
Date:   Tue Aug 29 12:03:29 2023 +0000

    debug snippet

    Signed-off-by: Anas Nashif <anas.nashif@intel.com>

diff --git a/snippets/debug/debug.conf b/snippets/debug/debug.conf
new file mode 100644
index 0000000000..53c5adaac3
--- /dev/null
+++ b/snippets/debug/debug.conf
@@ -0,0 +1,5 @@
+CONFIG_DEBUG=y
+CONFIG_THREAD_NAME=y
+CONFIG_THREAD_ANALYZER=y
+CONFIG_THREAD_ANALYZER_AUTO=y
+CONFIG_THREAD_ANALYZER_RUN_UNLOCKED=y
diff --git a/snippets/debug/snippet.yml b/snippets/debug/snippet.yml
new file mode 100644
index 0000000000..9c6d9e78a0
--- /dev/null
+++ b/snippets/debug/snippet.yml
@@ -0,0 +1,3 @@
+name: debug
+append:
+  EXTRA_CONF_FILE: debug.conf

and used this with you PR, and it works fine.

Adds support for twister to require using snippets on tests Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

Adds 2 tests which check required_snippet functionality works. Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

Adds a note that twister now supports applying snippets Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

Adds information on the new required_snippets feature Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

golowanow · 2023-08-30T11:32:15Z

scripts/schemas/twister/testsuite-schema.yaml

@@ -145,6 +145,11 @@ mapping:
        matching: "all"
        sequence:
          - type: str
+      "required_snippets":


might it be just snippets ? or are there other types of snippets possible e.g. 'optional'. If so, then shouldn't it be snippets_required to follow the naming convention here ?

The "required" part is to make it clear that the test will only run with them. I don't envisage having support for optional snippets, they will either be required and applied or not available and test will not run

nashif

still do not see of example usage in zephyr, but thats fine, I can add this as a followup.

nordicjm requested a review from JordanYates August 3, 2023 10:29

JordanYates reviewed Aug 15, 2023

View reviewed changes

scripts/pylib/twister/twisterlib/testplan.py Outdated Show resolved Hide resolved

nordicjm force-pushed the twistersnippets branch 5 times, most recently from d9dc6c8 to a273402 Compare August 18, 2023 11:09

nordicjm marked this pull request as ready for review August 18, 2023 11:10

nordicjm requested review from mbolivar-ampere and nashif as code owners August 18, 2023 11:10

zephyrbot added area: West West utility area: Twister Twister labels Aug 18, 2023

zephyrbot requested review from carlescufi, galak, gchwier, golowanow, gopiotr, hakehuang, PerMac and swinslow August 18, 2023 11:11

zephyrbot assigned nashif Aug 18, 2023

nordicjm force-pushed the twistersnippets branch 2 times, most recently from 56da98d to fb24b48 Compare August 18, 2023 11:34

zephyrbot added the Release Notes To be mentioned in the release notes label Aug 18, 2023

zephyrbot requested review from fabiobaltieri and jhedberg August 18, 2023 11:35

hakehuang reviewed Aug 20, 2023

View reviewed changes

doc/releases/release-notes-3.5.rst Show resolved Hide resolved

hakehuang previously approved these changes Aug 24, 2023

View reviewed changes

nordicjm dismissed hakehuang’s stale review via 8992224 August 24, 2023 10:50

nordicjm force-pushed the twistersnippets branch from a688bc8 to 8992224 Compare August 24, 2023 10:50

nordicjm requested a review from hakehuang August 24, 2023 10:52

nordicjm force-pushed the twistersnippets branch from 8992224 to a692c8f Compare August 24, 2023 10:53

hakehuang previously approved these changes Aug 25, 2023

View reviewed changes

nashif reviewed Aug 25, 2023

View reviewed changes

PerMac requested changes Aug 25, 2023

View reviewed changes

mbolivar-ampere reviewed Aug 28, 2023

View reviewed changes

nordicjm dismissed hakehuang’s stale review via e788e7b August 29, 2023 12:25

nordicjm force-pushed the twistersnippets branch from a692c8f to e788e7b Compare August 29, 2023 12:25

nordicjm added 4 commits August 29, 2023 14:36

twister: Add support for required snippets

ccb50e3

Adds support for twister to require using snippets on tests Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

tests: twister: Add tests for required_snippets functionality

154f043

Adds 2 tests which check required_snippet functionality works. Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

doc: release: 3.5: Add note on required_snippets twister addition

a496301

Adds a note that twister now supports applying snippets Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

doc: twister: Add details on required_snippets

933d26b

Adds information on the new required_snippets feature Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

nordicjm force-pushed the twistersnippets branch from e788e7b to 933d26b Compare August 29, 2023 13:37

nordicjm requested a review from PerMac August 29, 2023 14:55

PerMac approved these changes Aug 30, 2023

View reviewed changes

nordicjm requested review from mbolivar-ampere and hakehuang August 30, 2023 10:16

golowanow reviewed Aug 30, 2023

View reviewed changes

nashif approved these changes Sep 6, 2023

View reviewed changes

carlescufi merged commit e6d5b3f into zephyrproject-rtos:main Sep 6, 2023
32 checks passed

nordicjm deleted the twistersnippets branch December 12, 2023 14:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

twister: Add support for required snippets #61111

twister: Add support for required snippets #61111

nordicjm commented Aug 3, 2023

nordicjm commented Aug 3, 2023

nordicjm commented Aug 15, 2023

JordanYates left a comment

nordicjm commented Aug 15, 2023

nashif Aug 25, 2023

nashif left a comment

nordicjm commented Aug 25, 2023

PerMac left a comment

PerMac Aug 25, 2023

nordicjm Aug 29, 2023

mbolivar-ampere left a comment

mbolivar-ampere Aug 28, 2023

nashif commented Aug 28, 2023

nordicjm commented Aug 29, 2023 •

edited

Loading

PerMac commented Aug 29, 2023

nashif commented Aug 29, 2023 •

edited

Loading

golowanow Aug 30, 2023

nordicjm Aug 31, 2023

nashif left a comment

		@@ -238,6 +238,22 @@ def process_snippets(args: argparse.Namespace) -> Snippets:

		return snippets

		def process_snippets_inter(requested_snippets, snippet_roots) -> Snippets:

twister: Add support for required snippets #61111

twister: Add support for required snippets #61111

Conversation

nordicjm commented Aug 3, 2023

nordicjm commented Aug 3, 2023

nordicjm commented Aug 15, 2023

JordanYates left a comment

Choose a reason for hiding this comment

nordicjm commented Aug 15, 2023

nashif Aug 25, 2023

Choose a reason for hiding this comment

nashif left a comment

Choose a reason for hiding this comment

nordicjm commented Aug 25, 2023

PerMac left a comment

Choose a reason for hiding this comment

PerMac Aug 25, 2023

Choose a reason for hiding this comment

nordicjm Aug 29, 2023

Choose a reason for hiding this comment

mbolivar-ampere left a comment

Choose a reason for hiding this comment

mbolivar-ampere Aug 28, 2023

Choose a reason for hiding this comment

nashif commented Aug 28, 2023

nordicjm commented Aug 29, 2023 • edited Loading

PerMac commented Aug 29, 2023

nashif commented Aug 29, 2023 • edited Loading

golowanow Aug 30, 2023

Choose a reason for hiding this comment

nordicjm Aug 31, 2023

Choose a reason for hiding this comment

nashif left a comment

Choose a reason for hiding this comment

nordicjm commented Aug 29, 2023 •

edited

Loading

nashif commented Aug 29, 2023 •

edited

Loading