Skip to content

Commit

Permalink
scripts: twister: Add more Status assignment guards
Browse files Browse the repository at this point in the history
TwisterStatus now should not crash on wrong assignments
of type: TwisterStatus.NONEXISTENT_STATUS,
TwisterStatus['NONEXISTENT_STATUS'], and
TwisterStatus('NONEXISTENT_STATUS') and instead
fail the current test and go on.
Added three new Exception types.

Signed-off-by: Lukasz Mrugala <lukaszx.mrugala@intel.com>
  • Loading branch information
LukaszMrugala committed Dec 20, 2024
1 parent 0617bd3 commit 90672b3
Show file tree
Hide file tree
Showing 7 changed files with 168 additions and 32 deletions.
30 changes: 28 additions & 2 deletions scripts/pylib/twister/twisterlib/error.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,33 @@ class BuildError(TwisterException):
class ExecutionError(TwisterException):
pass

class StatusAttributeError(TwisterException):
class StatusAssignmentError(TwisterException):
def __init__(self, cls : type, value):
msg = f'{cls.__name__} assigned status {value}, which could not be cast to a TwisterStatus.'
msg = (
f'{cls.__name__} assigned status "{value}", which could not be cast to a TwisterStatus.'
)
super().__init__(msg)

class StatusInitError(TwisterException):
def __init__(self, value):
msg = (
f'Attempted initialisation of status "{value}",'
' which could not be cast to a TwisterStatus.'
)
super().__init__(msg)

class StatusAttributeError(AttributeError):
def __init__(self, name):
msg = (
f'Attempted access to nonexistent TwisterStatus.{name}.'
' Please verify the status list.'
)
super().__init__(msg)

class StatusKeyError(KeyError):
def __init__(self, name):
msg = (
f'Attempted access to nonexistent TwisterStatus[{name}].'
' Please verify the status list.'
)
super().__init__(msg)
4 changes: 2 additions & 2 deletions scripts/pylib/twister/twisterlib/harness.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from pytest import ExitCode
from twisterlib.constants import SUPPORTED_SIMS_IN_PYTEST
from twisterlib.environment import PYTEST_PLUGIN_INSTALLED, ZEPHYR_BASE
from twisterlib.error import ConfigurationError, StatusAttributeError
from twisterlib.error import ConfigurationError, StatusAssignmentError
from twisterlib.handlers import Handler, terminate_process
from twisterlib.reports import ReportStatus
from twisterlib.statuses import TwisterStatus
Expand Down Expand Up @@ -82,7 +82,7 @@ def status(self, value : TwisterStatus) -> None:
key = value.name if isinstance(value, Enum) else value
self._status = TwisterStatus[key]
except KeyError as err:
raise StatusAttributeError(self.__class__, value) from err
raise StatusAssignmentError(self.__class__, value) from err

def configure(self, instance):
self.instance = instance
Expand Down
72 changes: 57 additions & 15 deletions scripts/pylib/twister/twisterlib/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@
from packaging import version
from twisterlib.cmakecache import CMakeCache
from twisterlib.environment import canonical_zephyr_base
from twisterlib.error import BuildError, ConfigurationError, StatusAttributeError
from twisterlib.error import (
BuildError,
ConfigurationError,
StatusAssignmentError,
StatusAttributeError,
StatusInitError,
StatusKeyError
)
from twisterlib.statuses import TwisterStatus

if version.parse(elftools.__version__) < version.parse('0.24'):

Check failure on line 41 in scripts/pylib/twister/twisterlib/runner.py

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

Python lint error (I001) see https://docs.astral.sh/ruff/rules/unsorted-imports

scripts/pylib/twister/twisterlib/runner.py:7 Import block is un-sorted or un-formatted
Expand Down Expand Up @@ -972,8 +979,13 @@ def process(self, pipeline, done, message, lock, results):
next_op = 'report'
else:
next_op = 'cmake'
except StatusAttributeError as sae:
logger.error(str(sae))
except (
StatusAssignmentError,
StatusAttributeError,
StatusInitError,
StatusKeyError
) as se:
logger.error(str(se))
self.instance.status = TwisterStatus.ERROR
reason = 'Incorrect status assignment'
self.instance.reason = reason
Expand Down Expand Up @@ -1005,8 +1017,13 @@ def process(self, pipeline, done, message, lock, results):
next_op = 'report'
else:
next_op = 'build'
except StatusAttributeError as sae:
logger.error(str(sae))
except (
StatusAssignmentError,
StatusAttributeError,
StatusInitError,
StatusKeyError
) as se:
logger.error(str(se))
self.instance.status = TwisterStatus.ERROR
reason = 'Incorrect status assignment'
self.instance.reason = reason
Expand Down Expand Up @@ -1054,8 +1071,13 @@ def process(self, pipeline, done, message, lock, results):
next_op = 'report'
else:
next_op = 'gather_metrics'
except StatusAttributeError as sae:
logger.error(str(sae))
except (
StatusAssignmentError,
StatusAttributeError,
StatusInitError,
StatusKeyError
) as se:
logger.error(str(se))
self.instance.status = TwisterStatus.ERROR
reason = 'Incorrect status assignment'
self.instance.reason = reason
Expand Down Expand Up @@ -1085,8 +1107,13 @@ def process(self, pipeline, done, message, lock, results):
"Nowhere to run"
)
next_op = 'report'
except StatusAttributeError as sae:
logger.error(str(sae))
except (
StatusAssignmentError,
StatusAttributeError,
StatusInitError,
StatusKeyError
) as se:
logger.error(str(se))
self.instance.status = TwisterStatus.ERROR
reason = 'Incorrect status assignment'
self.instance.reason = reason
Expand All @@ -1111,8 +1138,13 @@ def process(self, pipeline, done, message, lock, results):
"status": self.instance.status,
"reason": self.instance.reason
}
except StatusAttributeError as sae:
logger.error(str(sae))
except (
StatusAssignmentError,
StatusAttributeError,
StatusInitError,
StatusKeyError
) as se:
logger.error(str(se))
self.instance.status = TwisterStatus.ERROR
reason = 'Incorrect status assignment'
self.instance.reason = reason
Expand Down Expand Up @@ -1140,8 +1172,13 @@ def process(self, pipeline, done, message, lock, results):
elif self.options.runtime_artifact_cleanup == "all":
next_op = 'cleanup'
additionals = {"mode": "all"}
except StatusAttributeError as sae:
logger.error(str(sae))
except (
StatusAssignmentError,
StatusAttributeError,
StatusInitError,
StatusKeyError
) as se:
logger.error(str(se))
self.instance.status = TwisterStatus.ERROR
reason = 'Incorrect status assignment'
self.instance.reason = reason
Expand All @@ -1161,8 +1198,13 @@ def process(self, pipeline, done, message, lock, results):
or (mode == "all" and self.instance.reason != "CMake build failure")
):
self.cleanup_artifacts()
except StatusAttributeError as sae:
logger.error(str(sae))
except (
StatusAssignmentError,
StatusAttributeError,
StatusInitError,
StatusKeyError
) as se:
logger.error(str(se))
self.instance.status = TwisterStatus.ERROR
reason = 'Incorrect status assignment'
self.instance.reason = reason
Expand Down
33 changes: 31 additions & 2 deletions scripts/pylib/twister/twisterlib/statuses.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,39 @@
"""
from __future__ import annotations

from enum import Enum
from enum import Enum, EnumMeta

from colorama import Fore
from twisterlib.error import (
StatusAttributeError,
StatusInitError,
StatusKeyError
)


class TwisterStatus(str, Enum):
class TwisterStatusEnumMeta(EnumMeta):

Check failure on line 20 in scripts/pylib/twister/twisterlib/statuses.py

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

Python lint error (I001) see https://docs.astral.sh/ruff/rules/unsorted-imports

scripts/pylib/twister/twisterlib/statuses.py:8 Import block is un-sorted or un-formatted
def __getattr__(cls, name):
try:
obj = super().__getattribute__(name)
except AttributeError as ae:
# Looks like an Enum key - let's give the user more information.
if (
name.isupper()
and not (name.startswith('_') and name.endswith('_'))
):
raise StatusAttributeError(name) from ae
else:
raise
return obj

def __getitem__(cls, name):
try:
return super().__getitem__(name)
except KeyError as ke:
raise StatusKeyError(name) from ke


class TwisterStatus(str, Enum, metaclass=TwisterStatusEnumMeta):
def __str__(self):
return str(self.value)

Expand All @@ -21,6 +48,8 @@ def _missing_(cls, value):
super()._missing_(value)
if value is None:
return TwisterStatus.NONE
elif value not in cls._value2member_map_:
raise StatusInitError(value)

@staticmethod
def get_color(status: TwisterStatus) -> str:
Expand Down
4 changes: 2 additions & 2 deletions scripts/pylib/twister/twisterlib/testinstance.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
SUPPORTED_SIMS_WITH_EXEC,
)
from twisterlib.environment import TwisterEnv
from twisterlib.error import BuildError, StatusAttributeError
from twisterlib.error import BuildError, StatusAssignmentError
from twisterlib.handlers import (
BinaryHandler,
DeviceHandler,
Expand Down Expand Up @@ -120,7 +120,7 @@ def status(self, value : TwisterStatus) -> None:
key = value.name if isinstance(value, Enum) else value
self._status = TwisterStatus[key]
except KeyError as err:
raise StatusAttributeError(self.__class__, value) from err
raise StatusAssignmentError(self.__class__, value) from err

def add_filter(self, reason, filter_type):
self.filters.append({'type': filter_type, 'reason': reason })
Expand Down
6 changes: 3 additions & 3 deletions scripts/pylib/twister/twisterlib/testsuite.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from pathlib import Path

from twisterlib.environment import canonical_zephyr_base
from twisterlib.error import StatusAttributeError, TwisterException, TwisterRuntimeError
from twisterlib.error import StatusAssignmentError, TwisterException, TwisterRuntimeError
from twisterlib.mixins import DisablePyTestCollectionMixin
from twisterlib.statuses import TwisterStatus

Expand Down Expand Up @@ -401,7 +401,7 @@ def status(self, value : TwisterStatus) -> None:
key = value.name if isinstance(value, Enum) else value
self._status = TwisterStatus[key]
except KeyError as err:
raise StatusAttributeError(self.__class__, value) from err
raise StatusAssignmentError(self.__class__, value) from err

def __lt__(self, other):
return self.name < other.name
Expand Down Expand Up @@ -469,7 +469,7 @@ def status(self, value : TwisterStatus) -> None:
key = value.name if isinstance(value, Enum) else value
self._status = TwisterStatus[key]
except KeyError as err:
raise StatusAttributeError(self.__class__, value) from err
raise StatusAssignmentError(self.__class__, value) from err

def load(self, data):
for k, v in data.items():
Expand Down
51 changes: 45 additions & 6 deletions scripts/tests/twister/test_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,19 @@
"""

import os
import re
import pytest

from pathlib import Path
from twisterlib.error import StatusAttributeError
from twisterlib.error import ConfigurationError
from twisterlib.error import (
StatusAttributeError,
StatusAssignmentError,
StatusInitError,
StatusKeyError,
ConfigurationError
)
from twisterlib.harness import Test
from twisterlib.statuses import TwisterStatus


def test_configurationerror():
Expand All @@ -25,11 +32,43 @@ def test_configurationerror():
raise ConfigurationError(cfile, message)


def test_status_value_error():
def test_status_assignment_error():
harness = Test()

expected_err = 'Test assigned status OK,' \
' which could not be cast to a TwisterStatus.'
expected_err = 'Test assigned status "OK", which could not be cast to a TwisterStatus.'

with pytest.raises(StatusAttributeError, match=expected_err):
with pytest.raises(StatusAssignmentError, match=expected_err):
harness.status = "OK"


def test_status_attribute_error():
harness = Test()

expected_err = (
'Attempted access to nonexistent TwisterStatus.OK. Please verify the status list.'
)

with pytest.raises(StatusAttributeError, match=expected_err):
harness.status = TwisterStatus.OK


def test_status_init_error():
harness = Test()

expected_err = (
'Attempted initialisation of status "OK", which could not be cast to a TwisterStatus.'
)

with pytest.raises(StatusInitError, match=expected_err):
harness.status = TwisterStatus("OK")


def test_status_key_error():
harness = Test()

expected_err = re.escape(
'Attempted access to nonexistent TwisterStatus[OK]. Please verify the status list.'
)

with pytest.raises(StatusKeyError, match=expected_err):
harness.status = TwisterStatus["OK"]

0 comments on commit 90672b3

Please sign in to comment.