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.

Sign up for GitHub

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

Jump to bottom

scripts: twister: Add TestCase status and reason printing #78345

Merged
merged 1 commit into from
Sep 17, 2024
Merged

scripts: twister: Add TestCase status and reason printing #78345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions scripts/pylib/twister/twisterlib/runner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1111,6 +1111,13 @@ def report_out(self, results):
results.done, total_tests_width, total_to_do , instance.platform.name,
instance.testsuite.name, status, more_info))

if self.options.verbose > 1:
for tc in self.instance.testcases:
color = TwisterStatus.get_color(tc.status)
logger.info(f' {" ":<{total_tests_width+25+4}} {tc.name:<75} '
f'{color}{str.upper(tc.status.value):<12}{Fore.RESET}'
f'{" " + tc.reason if tc.reason else ""}')

if instance.status in [TwisterStatus.ERROR, TwisterStatus.FAIL]:
self.log_info_file(self.options.inline_logs)
else:
Expand Down
17 changes: 17 additions & 0 deletions scripts/pylib/twister/twisterlib/statuses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
"""
Status classes to be used instead of str statuses.
"""
from __future__ import annotations

from colorama import Fore
from enum import Enum


Expand All @@ -19,6 +21,21 @@ def _missing_(cls, value):
if value is None:
return TwisterStatus.NONE

@staticmethod
def get_color(status: TwisterStatus) -> str:
match(status):
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: If you use match keyword, than Twister requires Python 3.10. Therefore, using from __future__ import annotations does not have any sense (it enables some type annotations for lower versions of python)

Copy link
Collaborator

Choose a reason for hiding this comment

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

just mentioned that, because couple days ago @golowanow added PR to still have possibility to run Twister with Python 3.8: #77077
With match keyword it is not possible

Copy link
Member

Choose a reason for hiding this comment

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

I just ran into this on macOS:

❯ scripts/twister --help
ZEPHYR_BASE unset, using "/Users/johedber/src/zephyr/zephyr"
Traceback (most recent call last):
  File "/Users/johedber/src/zephyr/zephyr/scripts/twister", line 207, in <module>
    from twisterlib.twister_main import main
  File "/Users/johedber/src/zephyr/zephyr/scripts/pylib/twister/twisterlib/twister_main.py", line 16, in <module>
    from twisterlib.statuses import TwisterStatus
  File "/Users/johedber/src/zephyr/zephyr/scripts/pylib/twister/twisterlib/statuses.py", line 26
    match(status):
                  ^
SyntaxError: invalid syntax
❯ which python3
/usr/bin/python3
❯ python3 --version
Python 3.9.6

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nit: If you use match keyword, than Twister requires Python 3.10. Therefore, using from __future__ import annotations does not have any sense (it enables some type annotations for lower versions of python)

You are right - I've used it to change TwisterStatus type hint into Self - it seems I didn't upstream it. My bad.

When it comes to Python<3.10 support - Zephyr does not claim to support it since 3.7.0 LTS (Python 3.10 is listed as minimum requirement), so I do not pay older versions special consideration.

Copy link
Member

@golowanow golowanow Sep 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

When it comes to Python<3.10 support - Zephyr does not claim to support it since 3.7.0

the problem is confusing user experience to get SyntaxError just for twister --help now.

#78671

case TwisterStatus.PASS:
color = Fore.GREEN
case TwisterStatus.SKIP | TwisterStatus.FILTER | TwisterStatus.BLOCK:
color = Fore.YELLOW
case TwisterStatus.FAIL | TwisterStatus.ERROR:
color = Fore.RED
case TwisterStatus.STARTED | TwisterStatus.NONE:
color = Fore.MAGENTA
case _:
color = Fore.RESET
return color
Comment on lines +26 to +37
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 dictionary lookup simpler for this purpose ?
just thinking of something like

return status2color[status] if status in status2color else Fore.RESET

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It would be less concise, as I wouldn't be able to use ORs (|) in the dictionary definition.

Copy link
Member

@golowanow golowanow Sep 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It would be less concise, as I wouldn't be able to use ORs (|) in the dictionary definition.

hmm, less concise ? I think this is more pythonic way, and you can even have another palette (grayscale), set it off assigning None, etc.

status2color = { TwisterStatus.PASS: Fore.GREEN,
                 TwisterStatus.SKIP: Fore.YELLOW,
                 TwisterStatus.FILTER: Fore.YELLOW,
                 TwisterStatus.BLOCK: Fore.YELLOW,
                 TwisterStatus.FAIL: Fore.RED,
                 TwisterStatus.ERROR: Fore.RED,
                 TwisterStatus.STARTED: Fore.MAGENTA,
                 TwisterStatus.NONE: Fore.MAGENTA
}

besides, no problem, just imo.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe concise wasn't the right word - the Status-Colour relationship is a many-to-one relationship (many statuses can have the same colour), which is immediately apparent in match-case structure. Dictionary is an inherently one-to-one structure (one value for one key). This necessitates repeating colours (YELLOW three times, RED two, MAGENTA two) and it's a bit harder to get information of type "Which Statuses can be yellow?"

Copy link
Member

@golowanow golowanow Sep 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

and it's a bit harder to get information of type "Which Statuses can be yellow?"

yeah, in case of the match/case construct it is even harder: it needs to be executed for all statuses first to build status2color{} and filter it on values=='yellow'.


# All statuses below this comment can be used for TestCase
BLOCK = 'blocked'
STARTED = 'started'
Expand Down
10 changes: 7 additions & 3 deletions scripts/tests/twister/test_runner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2008,9 +2008,13 @@ def test_projectbuilder_report_out(
instance_mock.status = status
instance_mock.reason = 'dummy reason'
instance_mock.testsuite.name = 'dummy.testsuite.name'
instance_mock.testsuite.testcases = [mock.Mock() for _ in range(25)]
instance_mock.testcases = [mock.Mock() for _ in range(24)] + \
[mock.Mock(status=TwisterStatus.SKIP)]
skip_mock_tc = mock.Mock(status=TwisterStatus.SKIP, reason='?')
skip_mock_tc.name = '?'
unknown_mock_tc = mock.Mock(status=mock.Mock(value='?'), reason='?')
unknown_mock_tc.name = '?'
instance_mock.testsuite.testcases = [unknown_mock_tc for _ in range(25)]
instance_mock.testcases = [unknown_mock_tc for _ in range(24)] + \
[skip_mock_tc]
env_mock = mock.Mock()

pb = ProjectBuilder(instance_mock, env_mock, mocked_jobserver)
Expand Down
Loading