Skip to content

Conversation

@LukaszMrugala
Copy link
Contributor

Currently, Twister Statuses are only useful for the initiated. This is in opposition to the fact that they are the main thing end users take into account when checking their Twister run reports.

In order to make Statuses more useful for the end user, a new documentation page has been created, elucidating the full meaning of all Statuses.

@LukaszMrugala LukaszMrugala added In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on area: Documentation area: Twister Twister labels Oct 30, 2024
@LukaszMrugala LukaszMrugala force-pushed the lmrugalx_status_documentation branch 3 times, most recently from 79d414a to dcd43ab Compare October 30, 2024 13:52
@nashif nashif added this to the v4.0.0 milestone Oct 30, 2024
@LukaszMrugala LukaszMrugala removed the In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on label Oct 30, 2024
@LukaszMrugala LukaszMrugala marked this pull request as ready for review October 30, 2024 14:48
@zephyrbot zephyrbot added the area: Testsuite Testsuite label Oct 30, 2024
@SzymonRichert SzymonRichert requested a review from kartben October 31, 2024 11:15
@LukaszMrugala LukaszMrugala added the In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on label Nov 8, 2024
@LukaszMrugala LukaszMrugala force-pushed the lmrugalx_status_documentation branch 3 times, most recently from 0ad5050 to 69f7f5b Compare November 8, 2024 15:20
@LukaszMrugala LukaszMrugala removed the In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on label Nov 8, 2024
Copy link
Member

@golowanow golowanow left a comment

Choose a reason for hiding this comment

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

Sorry for so many notes - mostly to improve reader's experience (IMO).

Copy link
Member

Choose a reason for hiding this comment

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

would be nice to use these names consistently

Suggested change
- Suite, Case
- TestSuite, TestCase

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added both names to the Tip section and changed all Test Suite, Test Case, etc. into Suite, Case, etc., respectively.

Comment on lines 94 to 96
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
**In-code** and **In-text** columns should the names of a given status.
If one is not a Twister developer, the in-text name should suffice.
It is the name used in the JSON reports.
**In-code** and **In-report** are the naming contexts of a given status: the former is rather internal for Twister and appears in logs, whereas the latter is used in the JSON reports.

Comment on lines +105 to +113
Copy link
Member

Choose a reason for hiding this comment

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

with bullets

Suggested change
``NONE`` (A starting status for Cases and Suites) and
``STARTED`` (Indicating an in-progress Case).
- ``NONE`` - an initial status for Cases and Suites,
- ``STARTED`` - Indicating an in-progress Case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, will have to see how it interacts with the Note box.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't play nicely with the box, had to remove it.

Copy link
Member

Choose a reason for hiding this comment

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

note the proposed changes in the text itself.

Copy link
Member

Choose a reason for hiding this comment

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

Isn't this paragraph almost the same as what is in the 'Possible Twister Statuses' table and can be moved there ?
If a state meaning is different for TestSuite vs. TestCase they can be split into table columns.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While the first sentence, the general Status description, is very similar to the table, in this section I aimed to explain why some combinations are possible and others are not.
When a documentation reader encounters the combination table, I'd assume they'd like to ask why certain combinations are impossible. So this needs to be after the combination table.

Copy link
Member

@golowanow golowanow Nov 13, 2024

Choose a reason for hiding this comment

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

unfortunately, having two descriptions tend to diverge from each other not clarifying much,
for example FILTER:

  • A static or runtime filter has eliminated the test from the list of tests to use.
  • This status indicates that the whole Suite has been statically filtered out of a given Twister run. Thus, any Case within it should also have such a status.
    The second sentence don't mention runtime filtering and it is still vague why other combinations are impossible (if it is the goal to answer here), e.g. together with PASS - why Suite: PASS and Case: FILTER is impossible ? same for NOTRUN.

SKIP is another example as it explains only one combination, isn't it ?

@LukaszMrugala LukaszMrugala force-pushed the lmrugalx_status_documentation branch from 69f7f5b to cbf8102 Compare November 12, 2024 14:02
@LukaszMrugala LukaszMrugala added the In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on label Nov 12, 2024
@nashif
Copy link
Member

nashif commented Nov 12, 2024

image

how are such combos possible?

suite: fail case: not run
suite: pass case: not run
suite: not run: case: skip
suite: error case: not run

@LukaszMrugala LukaszMrugala force-pushed the lmrugalx_status_documentation branch from cbf8102 to 0f20a39 Compare November 12, 2024 15:30
@LukaszMrugala
Copy link
Contributor Author

how are such combos possible?

suite: fail case: not run suite: pass case: not run suite: not run: case: skip suite: error case: not run

Current implementation of Twister does not allow for such combinations. Mainly because the runnability is decided on a per-Suite level and runnability is the deciding factor for the NOTRUN status. It is not, however, theoretically impossible - if we ever check runnability for a singular Case, those could be proper combinations.

It would require significant Twister changes which are not planned, however, so I'll mark them as not proper.

Currently, Twister Statuses are only useful for
the initiated, save for the very basics.
This is in opposition to the fact
that they are the main thing end users take into
account when checking their Twister run reports.

In order to make Statuses more useful for the end user,
a new documentation page has been created,
elucidating the full meaning of all Statuses.

Signed-off-by: Lukasz Mrugala <[email protected]>
@LukaszMrugala LukaszMrugala force-pushed the lmrugalx_status_documentation branch from 0f20a39 to 2c09c9f Compare November 13, 2024 07:30
Comment on lines +17 to +18
In practice, most users will be interested in Statuses
of Instances and Cases after the conclusion of their Twister runs.
Copy link
Member

@golowanow golowanow Nov 13, 2024

Choose a reason for hiding this comment

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

Suggested change
In practice, most users will be interested in Statuses
of Instances and Cases after the conclusion of their Twister runs.

IMO it is better to remove this from here to allow more clarification of what 'instance' is from user's perspective later in the 'nomenclature' section below.

Comment on lines +47 to +52
``TestInstance``, also called Instance, is a Suite on some platform.
Twister typically reports its results for Instances,
despite them being called "Suites" there.
If a status is marked as applicable for Suites, it is also applicable for Instances.
As the distinction between them is not useful in this section,
whenever you read "Suite", assume the same for Instances.
Copy link
Member

Choose a reason for hiding this comment

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

Twister summary output uses test configurations executed on platforms for 'instances'

:header-rows: 1

* - In-code
- In-text
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- In-text
- In-report

Comment on lines +105 to +113
Copy link
Member

Choose a reason for hiding this comment

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

note the proposed changes in the text itself.

Copy link
Member

@golowanow golowanow Nov 13, 2024

Choose a reason for hiding this comment

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

unfortunately, having two descriptions tend to diverge from each other not clarifying much,
for example FILTER:

  • A static or runtime filter has eliminated the test from the list of tests to use.
  • This status indicates that the whole Suite has been statically filtered out of a given Twister run. Thus, any Case within it should also have such a status.
    The second sentence don't mention runtime filtering and it is still vague why other combinations are impossible (if it is the goal to answer here), e.g. together with PASS - why Suite: PASS and Case: FILTER is impossible ? same for NOTRUN.

SKIP is another example as it explains only one combination, isn't it ?

@mmahadevan108
Copy link
Contributor

@LukaszMrugala , release date is Nov 15. I would like to merge this in sometime today.

Copy link
Member

@golowanow golowanow left a comment

Choose a reason for hiding this comment

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

+1 let's have it as it is for now

@mmahadevan108 mmahadevan108 merged commit 7092335 into zephyrproject-rtos:main Nov 15, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Documentation area: Testsuite Testsuite area: Twister Twister In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants