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: Fix warnings & update some style (dependency injection/type hints). #77989

Merged
merged 6 commits into from
Sep 12, 2024
Merged

scripts: twister: Fix warnings & update some style (dependency injection/type hints). #77989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1427113
scripts: twister: check for deprecation notice for space-separated lists
ithinuel Jul 30, 2024
7dd4b00
scripts: twister: Solve deprecation notice when running tests
ithinuel Jul 30, 2024
49a847d
scripts: twisterlib: Fix warning about deprecation of truthiness
ithinuel Aug 30, 2024
98f0133
scripts: twister: TwisterEnv is always used with an options parameter
ithinuel Aug 30, 2024
4def3ce
scripts: twister: Use args rather than injecting members
ithinuel Aug 30, 2024
b61c5fc
scripts: twister: add type hint to twister_main.py
ithinuel Aug 30, 2024
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
2 changes: 1 addition & 1 deletion scripts/pylib/twister/twisterlib/config_parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def extract_fields_from_arg_list(target_fields: set, arg_list: Union[str, list])
# Move to other_fields
other_fields.append(field)

return extracted_fields, " ".join(other_fields)
return extracted_fields, other_fields
Copy link
Member

Choose a reason for hiding this comment

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

i suspect get_scenario() needs modifications too, to align with the return type change here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As far as I can tell, extra_args is expected to be a list (in yaml) so returning it as a list seems ok to me.
I coulding find other places than test_twister that check for the content of this parameter explicitly. I may have missed some relevant location, could you point me to some of them please?


class TwisterConfigParser:
"""Class to read testsuite yaml files with semantic checking
Expand Down
26 changes: 12 additions & 14 deletions scripts/pylib/twister/twisterlib/environment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#
# Copyright (c) 2018 Intel Corporation
# Copyright 2022 NXP
# Copyright (c) 2024 Arm Limited (or its affiliates). All rights reserved.
#
# SPDX-License-Identifier: Apache-2.0

import argparse
Expand Down Expand Up @@ -931,33 +933,29 @@ def strip_ansi_sequences(s: str) -> str:

class TwisterEnv:

def __init__(self, options=None, default_options=None) -> None:
def __init__(self, options, default_options=None) -> None:
self.version = "Unknown"
self.toolchain = None
self.commit_date = "Unknown"
self.run_date = None
self.options = options
self.default_options = default_options

if options and options.ninja:
if options.ninja:
self.generator_cmd = "ninja"
self.generator = "Ninja"
else:
self.generator_cmd = "make"
self.generator = "Unix Makefiles"
logger.info(f"Using {self.generator}..")

self.test_roots = options.testsuite_root if options else None
self.test_roots = options.testsuite_root

if options:
if not isinstance(options.board_root, list):
self.board_roots = [self.options.board_root]
else:
self.board_roots = self.options.board_root
self.outdir = os.path.abspath(options.outdir)
if not isinstance(options.board_root, list):
self.board_roots = [options.board_root]
else:
self.board_roots = None
self.outdir = None
self.board_roots = options.board_root
self.outdir = os.path.abspath(options.outdir)

self.snippet_roots = [Path(ZEPHYR_BASE)]
modules = zephyr_module.parse_modules(ZEPHYR_BASE)
Expand All @@ -984,14 +982,14 @@ def __init__(self, options=None, default_options=None) -> None:

self.hwm = None

self.test_config = options.test_config if options else None
self.test_config = options.test_config

self.alt_config_root = options.alt_config_root if options else None
self.alt_config_root = options.alt_config_root

def non_default_options(self) -> dict:
"""Returns current command line options which are set to non-default values."""
diff = {}
if not self.options or not self.default_options:
if not self.default_options:
return diff
dict_options = vars(self.options)
dict_default = vars(self.default_options)
Expand Down
42 changes: 22 additions & 20 deletions scripts/pylib/twister/twisterlib/handlers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
#
# Copyright (c) 2018-2022 Intel Corporation
# Copyright 2022 NXP
# Copyright (c) 2024 Arm Limited (or its affiliates). All rights reserved.
#
# SPDX-License-Identifier: Apache-2.0

import argparse
import logging
import math
import os
Expand All @@ -24,6 +27,7 @@
from twisterlib.error import TwisterException
from twisterlib.platform import Platform
from twisterlib.statuses import TwisterStatus
from typing import Optional

sys.path.insert(0, os.path.join(ZEPHYR_BASE, "scripts/pylib/build_helpers"))
from domains import Domains
Expand Down Expand Up @@ -69,11 +73,12 @@ def terminate_process(proc):


class Handler:
def __init__(self, instance, type_str="build"):
def __init__(self, instance, type_str: str, options: argparse.Namespace,
generator_cmd: Optional[str] = None, suite_name_check: bool = True):
"""Constructor

"""
self.options = None
self.options = options

self.run = False
self.type_str = type_str
Expand All @@ -87,8 +92,8 @@ def __init__(self, instance, type_str="build"):
self.build_dir = instance.build_dir
self.log = os.path.join(self.build_dir, "handler.log")
self.returncode = 0
self.generator_cmd = None
self.suite_name_check = True
self.generator_cmd = generator_cmd
self.suite_name_check = suite_name_check
self.ready = False

self.args = []
Expand Down Expand Up @@ -170,16 +175,18 @@ def get_default_domain_build_dir(self):


class BinaryHandler(Handler):
def __init__(self, instance, type_str):
def __init__(self, instance, type_str: str, options: argparse.Namespace, generator_cmd: Optional[str] = None,
suite_name_check: bool = True):
"""Constructor

@param instance Test Instance
"""
super().__init__(instance, type_str)
super().__init__(instance, type_str, options, generator_cmd, suite_name_check)

self.seed = None
self.extra_test_args = None
self.line = b""
self.binary: Optional[str] = None

def try_kill_process_by_pid(self):
if self.pid_fn:
Expand Down Expand Up @@ -359,12 +366,13 @@ def handle(self, harness):


class SimulationHandler(BinaryHandler):
def __init__(self, instance, type_str):
def __init__(self, instance, type_str: str, options: argparse.Namespace, generator_cmd: Optional[str] = None,
suite_name_check: bool = True):
"""Constructor

@param instance Test Instance
"""
super().__init__(instance, type_str)
super().__init__(instance, type_str, options, generator_cmd, suite_name_check)

if type_str == 'renode':
self.pid_fn = os.path.join(instance.build_dir, "renode.pid")
Expand All @@ -374,14 +382,6 @@ def __init__(self, instance, type_str):


class DeviceHandler(Handler):

def __init__(self, instance, type_str):
"""Constructor

@param instance Test Instance
"""
super().__init__(instance, type_str)

def get_test_timeout(self):
timeout = super().get_test_timeout()
if self.options.enable_coverage:
Expand Down Expand Up @@ -809,13 +809,14 @@ class QEMUHandler(Handler):
for these to collect whether the test passed or failed.
"""

def __init__(self, instance, type_str):
def __init__(self, instance, type_str: str, options: argparse.Namespace, generator_cmd: Optional[str] = None,
suite_name_check: bool = True):
"""Constructor

@param instance Test instance
"""

super().__init__(instance, type_str)
super().__init__(instance, type_str, options, generator_cmd, suite_name_check)
self.fifo_fn = os.path.join(instance.build_dir, "qemu-fifo")

self.pid_fn = os.path.join(instance.build_dir, "qemu.pid")
Expand Down Expand Up @@ -1110,13 +1111,14 @@ class QEMUWinHandler(Handler):
for these to collect whether the test passed or failed.
"""

def __init__(self, instance, type_str):
def __init__(self, instance, type_str: str, options: argparse.Namespace, generator_cmd: Optional[str] = None,
suite_name_check: bool = True):
"""Constructor

@param instance Test instance
"""

super().__init__(instance, type_str)
super().__init__(instance, type_str, options, generator_cmd, suite_name_check)
self.pid_fn = os.path.join(instance.build_dir, "qemu.pid")
self.fifo_fn = os.path.join(instance.build_dir, "qemu-fifo")
self.pipe_handle = None
Expand Down
3 changes: 2 additions & 1 deletion scripts/pylib/twister/twisterlib/harness.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,8 @@ def _update_test_status(self):
def _parse_report_file(self, report):
tree = ET.parse(report)
root = tree.getroot()
if elem_ts := root.find('testsuite'):

if (elem_ts := root.find('testsuite')) is not None:
if elem_ts.get('failures') != '0':
self.status = TwisterStatus.FAIL
self.instance.reason = f"{elem_ts.get('failures')}/{elem_ts.get('tests')} pytest scenario(s) failed"
Expand Down
24 changes: 13 additions & 11 deletions scripts/pylib/twister/twisterlib/testinstance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
#
# Copyright (c) 2018-2022 Intel Corporation
# Copyright 2022 NXP
# Copyright (c) 2024 Arm Limited (or its affiliates). All rights reserved.
#
# SPDX-License-Identifier: Apache-2.0
from __future__ import annotations
from enum import Enum
Expand All @@ -13,6 +15,7 @@
import glob
import csv

from twisterlib.environment import TwisterEnv
from twisterlib.testsuite import TestCase, TestSuite
from twisterlib.platform import Platform
from twisterlib.error import BuildError, StatusAttributeError
Expand Down Expand Up @@ -201,41 +204,40 @@ def testsuite_runnable(testsuite, fixtures):

return can_run

def setup_handler(self, env):
def setup_handler(self, env: TwisterEnv):
# only setup once.
if self.handler:
return

options = env.options
handler = Handler(self, "")
common_args = (options, env.generator_cmd, not options.disable_suite_name_check)
if options.device_testing:
handler = DeviceHandler(self, "device")
handler = DeviceHandler(self, "device", *common_args)
handler.call_make_run = False
handler.ready = True
elif self.platform.simulation != "na":
if self.platform.simulation == "qemu":
if os.name != "nt":
handler = QEMUHandler(self, "qemu")
handler = QEMUHandler(self, "qemu", *common_args)
else:
handler = QEMUWinHandler(self, "qemu")
handler = QEMUWinHandler(self, "qemu", *common_args)
handler.args.append(f"QEMU_PIPE={handler.get_fifo()}")
handler.ready = True
else:
handler = SimulationHandler(self, self.platform.simulation)
handler = SimulationHandler(self, self.platform.simulation, *common_args)

if self.platform.simulation_exec and shutil.which(self.platform.simulation_exec):
handler.ready = True
elif self.testsuite.type == "unit":
handler = BinaryHandler(self, "unit")
handler = BinaryHandler(self, "unit", *common_args)
handler.binary = os.path.join(self.build_dir, "testbinary")
if options.enable_coverage:
handler.args.append("COVERAGE=1")
handler.call_make_run = False
handler.ready = True
else:
handler = Handler(self, "", *common_args)

if handler:
handler.options = options
handler.generator_cmd = env.generator_cmd
handler.suite_name_check = not options.disable_suite_name_check
self.handler = handler

# Global testsuite parameters
Expand Down
3 changes: 2 additions & 1 deletion scripts/pylib/twister/twisterlib/twister_main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# Copyright (c) 2022 Google
# SPDX-License-Identifier: Apache-2.0

import argparse
import colorama
import logging
import os
Expand Down Expand Up @@ -63,7 +64,7 @@ def init_color(colorama_strip):
colorama.init(strip=colorama_strip)


def main(options, default_options):
def main(options: argparse.Namespace, default_options: argparse.Namespace):
start_time = time.time()

# Configure color output
Expand Down
Loading
Loading