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

twister: support test plans and test configuration #52715

Merged
merged 2 commits into from Mar 7, 2023

Conversation

nashif
Copy link
Member

@nashif nashif commented Dec 1, 2022

Add support test plans and the ability to assign a specific test to one
or more plans. Use command line options of twister it is then possible
to select a plan and just execute the tests included in this plan.

Additionally, a test configuration is now available to define plan
dependencies and additional inclusion of tests into a specific plan if
the test itself does not have this information already.

In the configuration file you can include complete components using
regular expressions and you can specific which test plan to import from
the same file, making management of plans easier.

To help with testing outside of upstream CI infrastructure, additional
options are available in the configuration file, which can be hosted
locally. As of now, those options are available:

  • Ability to ignore default platforms as defined in board definitions
    (Those are mostly emulation platforms used to run tests in upstream
    CI)
  • Option to specify your own list of default platforms overriding what
    upstream defines.
  • Ability to override build_onl_all options used in some testscases.
    This will treat tests or sample as any other just build for default
    platforms you specify in the configuation file or on the command line.

commits:

  • twister: add support for plans and test configuration
  • tests: add smoke tests
  • tests: mem_map: remove scnearios used for coverage
  • tests: mem_protect: cleanup yaml and reenable qemu_arc

@@ -121,6 +179,11 @@ def discover(self):
raise TwisterRuntimeError("No test cases found at the specified location...")

self.find_subtests()
# get list of scenarios we have parsed into one list
for _, ts in self.testsuites.items():
self.scenarios.append(ts.id)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need check the uniq id here? I guess same cases maybe include multiply times

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I guess we can do the uniqueness check here.

@nashif nashif marked this pull request as ready for review January 30, 2023 16:11
@zephyrbot zephyrbot added the area: Twister Twister label Jan 30, 2023
@teburd
Copy link
Collaborator

teburd commented Feb 23, 2023

@PerMac @hakehuang

hakehuang
hakehuang previously approved these changes Mar 2, 2023
teburd
teburd previously approved these changes Mar 2, 2023
@PerMac
Copy link
Member

PerMac commented Mar 3, 2023

I am finalizing my review, please wait with merging.

@carlescufi carlescufi added the DNM This PR should not be merged (Do Not Merge) label Mar 3, 2023
@carlescufi
Copy link
Member

DNM waiting for a review from @PerMac

Copy link
Member

@PerMac PerMac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I think this is a very useful feature. However, I have some objections.
Comments added. Additionally, since this is a new feature a proper documentation is IMO a must. I tested some workflows but often had to guess what is the intended behavior.

scripts/pylib/twister/twisterlib/environment.py Outdated Show resolved Hide resolved
scripts/pylib/twister/twisterlib/testplan.py Outdated Show resolved Hide resolved
required: false
sequence:
- type: str
enum: ["smoke", "unit", "integration", "acceptance", "sanity", "functional"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why such strict list of possible names? It seems rather arbitrary. E.g. according to one of testing standards smoke and sanity are synonyms https://glossary.istqb.org/en_US/term/smoke-test-1 (I know it can mean something slightly different for some people, but that's why defining standard, mentioned later on, is so important). I also recall that we had to change the name of twister from sanitycheck since "sanity" is not inclusive.
The names are suggesting reference to testing levels but there is also "functional" (type of a test, why there is no "non-functional" then?) and "smoke" (type of a test suite) in the list, which doesn't refer to any level. And this field is called "plans".
Why not allowing "pr", "push", "schedule" if we are talking about plans (scopes), like it is used in gh workflow? Or "nightly" and "weekly" which I believe are also commonly used to refer to a testing scope?

I think this mixes too many things in one place. I am fine with adding extra descriptions to tests referring to their level. But then

Then a test plan could ask e.g. for all tests at "unit" and "integration" level. Or if we add other keyword (e.g. functional vs non-functional) argument to also look for such descriptions.

I am also fine with not having such enum validation in this iteration.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why such strict list of possible names?

The list can always be expanded. Otherwise we will end up with a mess with custom levels and things will be missed. This is a list of possible levels, does not mean you have to use them all, what fall under each of those is more or less well defined in the testing universe.

This change has been there for 3 months and the list was open for discussion, but nobody complained. We can modify it expand on it, I am fine with that, but we need to have this pre-defined to avoid random levels going in that will be difficult to manage later.

pr, push, schedule are not testing levels, those are events and everyone can decide what to run and when based on their needs.

The confusion here is coming most likely from plan vs level, I will change everything back to level as it was before, but the schema will have to remain, maybe modifed a bit and following some standard names and what we need as a project.

scripts/pylib/twister/twisterlib/testplan.py Outdated Show resolved Hide resolved
scripts/pylib/twister/twisterlib/testplan.py Outdated Show resolved Hide resolved
scripts/pylib/twister/twisterlib/testplan.py Outdated Show resolved Hide resolved
tests/test_config.yaml Outdated Show resolved Hide resolved
@carlescufi carlescufi removed the DNM This PR should not be merged (Do Not Merge) label Mar 3, 2023
@nashif nashif dismissed stale reviews from teburd and hakehuang via 7080f43 March 6, 2023 07:11
@nashif nashif force-pushed the topic/twister/levels branch 3 times, most recently from 386bcce to afa31c3 Compare March 6, 2023 07:42
@nashif nashif force-pushed the topic/twister/levels branch 3 times, most recently from c40d64b to 01198a0 Compare March 6, 2023 08:22
@nashif nashif requested a review from PerMac March 6, 2023 08:29
hakehuang
hakehuang previously approved these changes Mar 7, 2023
Copy link
Collaborator

@hakehuang hakehuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the test level feature.

PerMac
PerMac previously approved these changes Mar 7, 2023
Copy link
Member

@PerMac PerMac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just pointing 2 minor typos. I don't mind having them fixed latter on.

doc/develop/test/twister.rst Outdated Show resolved Hide resolved
scripts/pylib/twister/twisterlib/environment.py Outdated Show resolved Hide resolved
Add support test levels and the ability to assign a specific test to one
or more levels. Using command line options of twister it is then possible
to select a level and just execute the tests included in this level.

Additionally, a test configuration allows definiing level
dependencies and additional inclusion of tests into a specific level if
the test itself does not have this information already.

In the configuration file you can include complete components using
regular expressions and you can specify which test level to import from
the same file, making management of levels easier.

To help with testing outside of upstream CI infrastructure, additional
options are available in the configuration file, which can be hosted
locally. As of now, those options are available:

- Ability to ignore default platforms as defined in board definitions
  (Those are mostly emulation platforms used to run tests in upstream
  CI)
- Option to specify your own list of default platforms overriding what
  upstream defines.
- Ability to override build_onl_all options used in some testscases.
  This will treat tests or sample as any other just build for default
  platforms you specify in the configuation file or on the command line.

Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Document test configurations and levels.

Signed-off-by: Anas Nashif <anas.nashif@intel.com>
@nashif nashif dismissed stale reviews from PerMac and hakehuang via 6f9b0f3 March 7, 2023 11:58
@carlescufi carlescufi merged commit f4dc918 into zephyrproject-rtos:main Mar 7, 2023
27 checks passed
PerMac added a commit to PerMac/zephyr that referenced this pull request May 5, 2023
A change to the scope selection rules was introduced by zephyrproject-rtos#52715
but it doesn't comply with the description in the code. What's more,
the change introduces major issues i.e. non deterministic scope
selection, especially in a CI environment. More context at zephyrproject-rtos#57595

Signed-off-by: Maciej Perkowski <Maciej.Perkowski@nordicsemi.no>
carlescufi pushed a commit that referenced this pull request May 8, 2023
A change to the scope selection rules was introduced by #52715
but it doesn't comply with the description in the code. What's more,
the change introduces major issues i.e. non deterministic scope
selection, especially in a CI environment. More context at #57595

Signed-off-by: Maciej Perkowski <Maciej.Perkowski@nordicsemi.no>
rlubos pushed a commit to rlubos/zephyr that referenced this pull request May 9, 2023
…m_allow

A change to the scope selection rules was introduced by zephyrproject-rtos#52715
but it doesn't comply with the description in the code. What's more,
the change introduces major issues i.e. non deterministic scope
selection, especially in a CI environment. More context at zephyrproject-rtos#57595

(cherry picked from commit 3bf7f83)
Signed-off-by: Maciej Perkowski <Maciej.Perkowski@nordicsemi.no>
qipengzha pushed a commit to qipengzha/zephyr that referenced this pull request May 24, 2023
A change to the scope selection rules was introduced by zephyrproject-rtos#52715
but it doesn't comply with the description in the code. What's more,
the change introduces major issues i.e. non deterministic scope
selection, especially in a CI environment. More context at zephyrproject-rtos#57595

Signed-off-by: Maciej Perkowski <Maciej.Perkowski@nordicsemi.no>
qipengzha pushed a commit to qipengzha/zephyr that referenced this pull request May 24, 2023
A change to the scope selection rules was introduced by zephyrproject-rtos#52715
but it doesn't comply with the description in the code. What's more,
the change introduces major issues i.e. non deterministic scope
selection, especially in a CI environment. More context at zephyrproject-rtos#57595

Signed-off-by: Maciej Perkowski <Maciej.Perkowski@nordicsemi.no>
aescolar pushed a commit to aescolar/zephyr that referenced this pull request Jun 1, 2023
…m_allow

A change to the scope selection rules was introduced by zephyrproject-rtos#52715
but it doesn't comply with the description in the code. What's more,
the change introduces major issues i.e. non deterministic scope
selection, especially in a CI environment. More context at zephyrproject-rtos#57595

(cherry picked from commit 3bf7f83)
Signed-off-by: Maciej Perkowski <Maciej.Perkowski@nordicsemi.no>
(cherry picked from commit f405f78)
canisLupus1313 pushed a commit that referenced this pull request Jul 4, 2023
…m_allow

A change to the scope selection rules was introduced by #52715
but it doesn't comply with the description in the code. What's more,
the change introduces major issues i.e. non deterministic scope
selection, especially in a CI environment. More context at #57595

(cherry picked from commit 3bf7f83)
Signed-off-by: Maciej Perkowski <Maciej.Perkowski@nordicsemi.no>
(cherry picked from commit f405f78)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants