Rahix / tbot

Automation/Testing tool for Embedded Linux Development
https://tbot.tools
GNU General Public License v3.0
84 stars 21 forks source link

Early load config when integrating tbot into pytest #85

Closed mebel-christ closed 1 year ago

mebel-christ commented 1 year ago

I am currently working on a test setup with tbot + pytest, but I am facing some challenges with the proposed skeleton integration. As you know the execution of pytest can be roughly boiled down to the following two stages:

  1. Collection stage: pytest searches all the tests, parameters, fixtures,... that make up the test configurations
  2. Execution stage: pytest actually executes the collected test configurations

By design pytest does not execute fixtures during collection stage which in turn makes it impossible to interact with tbot during that stage since the tbot integration is solely done as a fixture. I want to propose an alternative integration mechanism to make it possible to interact with tbot during collection stage

Use case

First up I want to outline my use case why I want to interact with tbot during collection stage in the first place with the following (minimal) working example. I have several configuration files that may look like this:

import tbot
from tbot.machine import board, connector

class DummyBoard(connector.SubprocessConnector, board.Board):

    @property
    def interfaces(self):
        return ["interface0", "interface1"]

def register_machines(ctx):
    ctx.register(DummyBoard, tbot.role.Board)

I want to manage variable interface types and amounts at the board level. For that regard I use a property in the Board class that returns how many instances of a certain type this Board has. For simplicity reasons in this example the interface is modeled by a simple string.

Assume there is a test that checks whether a single interface (of a certain type) works as expected. Naturally I want to run this test for every single interface instance present on a Board. With pytest this is possible by using the pytest.mark.parametrize decorator like so:

import pytest
import tbot

def _interfaces():
    with tbot.ctx.request(tbot.role.Board) as b:
        return b.interfaces

@pytest.mark.parametrize("interface", _interfaces())
def test_interface(interface):
    print(interface)

With the currently described integration mechanism this results in tbot.error.MachineNotFoundError: no machine found for <class 'tbot.role.Board'> since parametrize runs during the collection stage and hence when requesting the Board context in _interfaces() the fixture that loads the config was not executed yet.

Proposal

To make this work I propose the following change to conftest.py:

import pytest
import tbot
from tbot import newbot

def pytest_addoption(parser, pluginmanager):
    parser.addoption("--tbot-config", action="append", default=[], dest="tbot_configs")
    parser.addoption("--tbot-flag", action="append", default=[], dest="tbot_flags")

def pytest_sessionstart(session):
    # Only register configuration when nobody else did so already.
    if not tbot.ctx.is_active():
        # Register flags
        for flag in session.config.option.tbot_flags:
            tbot.flags.add(flag)
        # Register configuration
        for config in session.config.option.tbot_configs:
            newbot.load_config(config, tbot.ctx)

@pytest.fixture(scope="session", autouse=True)
def tbot_ctx():
    with tbot.ctx:
        # Configure the context for keep_alive (so machines can be reused
        # between tests).  reset_on_error_by_default will make sure test
        # failures lead to a powercycle of the DUT anyway.
        with tbot.ctx.reconfigure(keep_alive=True, reset_on_error_by_default=True):
            # Tweak the standard output logging options
            with tbot.log.with_verbosity(tbot.log.Verbosity.STDOUT, nesting=1):
                yield tbot.ctx

Move the "configuration registration" step to the pytest_sessionstart hook. This loads the tbot configuration before the collection stage runs and makes it possible to parametrize a test in the described fashion:

$ pytest -v --tbot-config config.dummy
=================================================================== test session starts ====================================================================
...
collected 2 items                                                                                                                                          

tests/test_interface.py::test_interface[interface0] PASSED                                                                                           [ 50%]
tests/test_interface.py::test_interface[interface1] PASSED                                                                                           [100%]

==================================================================== 2 passed in 0.02s =====================================================================

Are there some Gotchas I am missing here? Otherwise I would happily supply a PR to the doc. Feedback on that approach is greatly appreciated.

Rahix commented 1 year ago

Hey, thanks, this is a very good idea! I think what you're proposing makes a lot of sense. Can you send a PR with a change to the docs and also selftest/conftest.py for it? :)

I have one comment about the example you make:

import pytest
import tbot

def _interfaces():
    with tbot.ctx.request(tbot.role.Board) as b:
        return b.interfaces

@pytest.mark.parametrize("interface", _interfaces())
def test_interface(interface):
    print(interface)

I don't think you should do a request() here for collecting the available interfaces. This will try initializing the board, so potentially power it on only to power it off immediately after. A better way would be to do this:

def _interfaces():
    board_cls = tbot.ctx.get_machine_class(tbot.role.Board)
    return board_cls.interfaces

This will not attempt interacting with a real board instance and just retrive the "cold" data from the class definition.

mebel-christ commented 1 year ago

I am working on the PR. Additionally I did some further testing:

I have one comment about the example you make:

import pytest
import tbot

def _interfaces():
    with tbot.ctx.request(tbot.role.Board) as b:
        return b.interfaces

@pytest.mark.parametrize("interface", _interfaces())
def test_interface(interface):
    print(interface)

I don't think you should do a request() here for collecting the available interfaces. This will try initializing the board, so potentially power it on only to power it off immediately after.

This is a great hint I missed that since I am working on my local machine with the dummy setup and not the real lab.

A better way would be to do this:

def _interfaces():
    board_cls = tbot.ctx.get_machine_class(tbot.role.Board)
    return board_cls.interfaces

This will not attempt interacting with a real board instance and just retrive the "cold" data from the class definition

Your proposed improvement works perfectly well, but it has a slight drawback. I can no longer use/implement the interfaces in the board with @property since properties belong to an instance and not the class itself. But TBH since this is static information anyway I do not really care. So I went with the following instead, but this is rather off-topic:

from typing import Dict, Set

from tbot import role
from tbot.machine import board, connector

class DummyBoard(connector.SubprocessConnector, board.Board):

    interfaces: Dict[str, Set[str]] = {
        "interface": {"interface0", "interface1"},
    }

More importantly I moved the config load step for tbot to an even earlier point in time, namely the pytest_configure hook. This was done to make use of pytest-metadata (+ pytest-html) to read and print metadata from a Board, but the more important point IMO is what the pytest-doc has to say about the hook:

pytest_configure(config)

Allow plugins and conftest files to perform initial configuration. This hook is called for every plugin and initial conftest file after command line options have been parsed. After that, the hook is called for other conftest files as they are imported.

To me this sounds like the even more appropriate (and portable?) location (than pytest_sessionstart) for loading a configuration file. @Rahix What do you think?

Rahix commented 1 year ago

Agreed, pytest_configure() sounds better! :+1:

mebel-christ commented 1 year ago

@Rahix I forked and added the documentation for this. TBH I am uncertain what to do about selftest/conftest.py now. I looked at it and as far as I understand the configuration for the testmachines is not loaded in any related way or am I misunderstanding something here?

Rahix commented 1 year ago

Ah, you are right, the selftests do things a bit differently. In that case, ignore what I said, sorry!

mebel-christ commented 1 year ago

Great, in that case I am going to open the PR