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
twister: support test plans and test configuration #52715
Conversation
@@ -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) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
6413441
to
75113fe
Compare
75113fe
to
871ad67
Compare
I am finalizing my review, please wait with merging. |
DNM waiting for a review from @PerMac |
There was a problem hiding this 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.
required: false | ||
sequence: | ||
- type: str | ||
enum: ["smoke", "unit", "integration", "acceptance", "sanity", "functional"] |
There was a problem hiding this comment.
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
- a defined list from some existing standard has to be used (e.g. https://en.wikipedia.org/wiki/Software_testing#Testing_levels i.e. unit (component), integration, system, acceptance which is also what ISTQB defines http://istqbfoundation.blogspot.com/p/2.html).
- this is called as e.g. "level" (not "plans")
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.
There was a problem hiding this comment.
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.
871ad67
to
7080f43
Compare
386bcce
to
afa31c3
Compare
c40d64b
to
01198a0
Compare
There was a problem hiding this 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.
There was a problem hiding this 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.
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>
01198a0
to
6f9b0f3
Compare
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>
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>
…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>
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>
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>
…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)
…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)
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:
(Those are mostly emulation platforms used to run tests in upstream
CI)
upstream defines.
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: