Unverified Commit d587d569 authored by Maximilian Bosch's avatar Maximilian Bosch
Browse files

nixos/test-driver: restructure error classes 



After a discussion with tfc, we agreed that we need a distinction
between errors where the user isn't at fault (e.g. OCR failing - now
called `MachineError`) and errors where the test actually failed (now
called `RequestedAssertionFailed`).

Both get special treatment from the error handler, i.e. a `!!!` prefix
to make it easier to spot visually.

However, only `RequestedAssertionFailed` gets the shortening of the
traceback, `MachineError` exceptions may be something to report and
maintainers usually want to see the full trace.

Suggested-by: default avatarJacek Galowicz <jacek@galowicz.de>
parent cc3d409a

nixos/lib/test-driver/src/test_driver/driver.py

+6 −2
Original line number Diff line number Diff line
@@ -11,7 +11,7 @@ from pathlib import Path 
from typing import Any

from unittest import TestCase



from test_driver.errors import RequestedAssertionFailed, TestScriptError

from test_driver.errors import MachineError, RequestedAssertionFailed

from test_driver.logger import AbstractLogger

from test_driver.machine import Machine, NixStartScript, retry

from test_driver.polling_condition import PollingCondition
@@ -182,7 +182,11 @@ class Driver: 
            symbols = self.test_symbols()  # call eagerly

            try:

                exec(self.tests, symbols, None)

            except TestScriptError:

            except MachineError:

                for line in traceback.format_exc().splitlines():

                    self.logger.log_test_error(line)

                sys.exit(1)

            except RequestedAssertionFailed:

                exc_type, exc, tb = sys.exc_info()

                filtered = [

                    frame

nixos/lib/test-driver/src/test_driver/errors.py

+12 −15
Original line number Diff line number Diff line 
class TestScriptError(Exception):

class MachineError(Exception):

    """

    The base error class to indicate that the test script failed.

    This (and its subclasses) get special treatment, i.e. only stack

    frames from `testScript` are printed and the error gets prefixed

    with `!!!` to make it easier to spot between other log-lines.

    Exception that indicates an error that is NOT the user's fault,

    i.e. something went wrong without the test being necessarily invalid,

    such as failing OCR.



    This class is used for errors that aren't an actual test failure,

    but also not a bug in the driver, e.g. failing OCR.

    To make it easier to spot, this exception (and its subclasses)

    get a `!!!` prefix in the log output.

    """





class RequestedAssertionFailed(TestScriptError):

class RequestedAssertionFailed(AssertionError):

    """

    Subclass of `TestScriptError` that gets special treatment.

    Special assertion that gets thrown on an assertion error,

    e.g. a failing `t.assertEqual(...)` or `machine.succeed(...)`.



    Exception raised when a requested assertion fails,

    e.g. `machine.succeed(...)` or `t.assertEqual(...)`.



    This is separate from the base error class, to have a dedicated class name

    that better represents this kind of failures.

    (better readability in test output)

    This gets special treatment in error reporting: i.e. it gets

    `!!!` as prefix just as `MachineError`, but all stack frames that are

    not from `testScript` also get removed.

    """

nixos/lib/test-driver/src/test_driver/machine.py

+5 −5
Original line number Diff line number Diff line
@@ -18,7 +18,7 @@ from pathlib import Path 
from queue import Queue

from typing import Any



from test_driver.errors import RequestedAssertionFailed, TestScriptError

from test_driver.errors import MachineError, RequestedAssertionFailed

from test_driver.logger import AbstractLogger



from .qmp import QMPSession
@@ -129,7 +129,7 @@ def _preprocess_screenshot(screenshot_path: str, negate: bool = False) -> str: 
    )



    if ret.returncode != 0:

        raise TestScriptError(

        raise MachineError(

            f"Image processing failed with exit code {ret.returncode}, stdout: {ret.stdout.decode()}, stderr: {ret.stderr.decode()}"

        )
@@ -140,7 +140,7 @@ def _perform_ocr_on_screenshot( 
    screenshot_path: str, model_ids: Iterable[int]

) -> list[str]:

    if shutil.which("tesseract") is None:

        raise TestScriptError("OCR requested but enableOCR is false")

        raise MachineError("OCR requested but enableOCR is false")



    processed_image = _preprocess_screenshot(screenshot_path, negate=False)

    processed_negative = _preprocess_screenshot(screenshot_path, negate=True)
@@ -163,7 +163,7 @@ def _perform_ocr_on_screenshot( 
                capture_output=True,

            )

            if ret.returncode != 0:

                raise TestScriptError(f"OCR failed with exit code {ret.returncode}")

                raise MachineError(f"OCR failed with exit code {ret.returncode}")

            model_results.append(ret.stdout.decode("utf-8"))



    return model_results
@@ -922,7 +922,7 @@ class Machine: 
            ret = subprocess.run(f"pnmtopng '{tmp}' > '{filename}'", shell=True)

            os.unlink(tmp)

            if ret.returncode != 0:

                raise TestScriptError("Cannot convert screenshot")

                raise MachineError("Cannot convert screenshot")



    def copy_from_host_via_shell(self, source: str, target: str) -> None:

        """Copy a file from the host into the guest by piping it over the