Skip to content

Introduce Broken Strategy State, Prevent Further Use and Skip Subsequent Tests #1681

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.

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Bastian-Krause
Copy link
Member

@Bastian-Krause Bastian-Krause commented Jul 3, 2025

Description
Introduce a decorator for the Strategy's transition() and force() methods. It stores the original exception in the Strategy's new broken attribute and raises a StrategyError from it. All subsequent calls to the decorated method will lead to the original exception.

Now that a Strategy can be marked broken easily, check for that in labgrid's pytest plugin before executing a test and its fixtures. If a broken Strategy is found in one of the targets, skip the test. It's not worth continuing with it. It only leads to cascading and confusing errors.

Having a strategy prepared with the new decorator, a strategy exception now leads to either a test error (when triggered in a fixture) or a test fail (when triggered in the test itself). All subsequent tests requesting the broken strategy (via the strategy fixture) will be skipped.

In order to get the word out about the @raise_if_broken decorator, decorate the Strategies in labgrid with it.

Previously this kind of workaround was often used:

@pytest.fixture
def shell(strategy, target):
    try:
        strategy.transition("shell")
        return target.get_driver("ShellDriver")
    except Exception:
        pytest.exit("Strategy failed")

This is now obsolete.

Checklist

  • Documentation for the feature
  • Tests for the feature (covered in tests/test_docker.py::test_docker_without_daemon)
  • Add a section on how to use the feature to doc/usage.rst
  • Add a section on how to use the feature to doc/development.rst
  • PR has been tested

Related to #1123.

Copy link

codecov bot commented Jul 3, 2025

Codecov Report

Attention: Patch coverage is 80.55556% with 7 lines in your changes missing coverage. Please review.

Project coverage is 55.7%. Comparing base (e43d093) to head (0815740).
Report is 9 commits behind head on master.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
labgrid/pytestplugin/hooks.py 40.0% 6 Missing ⚠️
labgrid/strategy/common.py 92.3% 1 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##           master   #1681   +/-   ##
======================================
  Coverage    55.6%   55.7%           
======================================
  Files         172     172           
  Lines       13466   13501   +35     
======================================
+ Hits         7492    7523   +31     
- Misses       5974    5978    +4     
Flag Coverage Δ
3.10 55.7% <80.5%> (+<0.1%) ⬆️
3.11 55.7% <80.5%> (+<0.1%) ⬆️
3.12 55.7% <80.5%> (+<0.1%) ⬆️
3.13 55.6% <80.5%> (+<0.1%) ⬆️
3.9 55.7% <80.5%> (+<0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This is the first step to broken state tracking for strategies.
A future commit will add a decorator to be used on the the strategy's
transition()/force() method that actually sets the exception and raises
it for subsequent calls

Signed-off-by: Bastian Krause <[email protected]>
Introduce a decorator meant to decorate the strategy's transition() and
force() methods. It stores the original exception in the broken
attribute and raises a StrategyError from it. All subsequent calls to the
decorated method will lead to the same exception.

This prevents a strategy from subsequent use after an exception
happened, thus no cascading errors can happen.

Signed-off-by: Bastian Krause <[email protected]>
…_setup()

Now that a strategy can be marked broken, check for that before
executing a test and its fixtures. If a broken strategy is found in one
of the targets, skip the test. It's not worth continuing with it. It
only leads to cascading errors.

The check for brokenness cannot be performed in the strategy fixture
itself because it's session scoped, meaning it only runs for the first
test requesting it (directly or indirectly). So wrap pytest's
pytest_runtest_setup hook running before fixture/test exexcution, Check
if the test requested the strategy fixture (directly or indirectly) and
perform the check only in that case.
Since we cannot know what target the strategy acts on (the target
fixture might be overridden by the test suite), simply check all targets
for broken strategies and skip the test if any of these have a broken
strategy.

This means a strategy exception leads to either a test error (when
triggered in a fixture) or a test fail (when triggered in the test
itself). All subsequent tests requesting the broken strategy (via the
strategy fixture) will be skipped.

Signed-off-by: Bastian Krause <[email protected]>
In order to get the word out about the @raise_if_broken decorator,
decorate the strategies in labgrid with it.

Signed-off-by: Bastian Krause <[email protected]>
In order to get the word out about the @raise_if_broken decorator,
decorate the strategies in labgrid with it.

Signed-off-by: Bastian Krause <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant