scenarios to return a class and avoid injection into locals

[youtux]

Reading the comment at #310 (comment) (and relative implementation), I figured out that we could make the scenarios(...) function much neater if we made it return a class instead of injecting into the module locals.

For example, given the following feature file:

Feature: My Feature
    Scenario: Scenario 1
    Scenario: Scenario 2
    Scenario: Scenario 3

and the following file:

# test_my_feature.py
from pytest_bdd import scenarios

TestMyFeature = scenarios("my_feature.feature")

and it would be syntactic sugar for the following:

# test_my_feature.py

# Not real code that should be typed, it's just the equivalent version of the file above
from pytest_bdd import scenarios

class TestMyFeature:
    @scenario
    test_scenario_1(): pass

    @scenario
    test_scenario_2(): pass

    @scenario
    test_scenario_3(): pass

This way, if a user needs to override a specific scenario for whatever reason, that could be done easily like this:

# test_my_feature.py
from pytest_bdd import scenarios

class TestMyFeature(scenarios("my_feature.py")):
    @scenario
    test_scenario_3(): pass

I can imagine an stub implementation of scenarios(...) to be something like this:

def scenarios(feature_path):
    ...
    feature = get_feature(feature_path)
    test_cls = type(camel_case(feature.name), (), {})
    for feature in get_features(feature_path):
        for scenario_name, scenario_object in feature.scenarios.items():
            @scenario(feature_path, scenario_name)
            def _scenario(self): pass
            setattr(test_cls, f"test_{snake_case(scenario_name)}", _scenario)
    return test_cls

[youtux]

@elchupanebrej I was inspired to have this change after reading your comment

[elchupanebrej]

Please pay attention, that unittest TestClasses have serious drawbacks when executed by pytest. But there could be special TestClass that could be collected by a specific Collector. I didn't succeed with my first attempts, I'll try to implement another PoC

[jsa34]

@youtux is this still something that you think would be helpful to revisit?

[youtux]

Maybe yes, although I have also get term plan to remove (or at least make optional) the calls to scenarios or @scenario, by instead converting .feature files into test modules themselves. Pytest has an example of how to do that for a YAML file, so I had in mind to follow the same principle.

If we have to put effort into a solution, I'd rather to this one I just mentioned instead.

[golikovichev]

+1 on revisiting this, with a concrete reason. The thing that bites us with scenarios() is that the generated tests are invisible to static tooling. Linters flag the scenarios import as unused, there's no go-to-definition for the generated tests, and you can't tell at a glance what got collected from a feature file. A class would at least give editors and linters something real to point at.

The override case comes up for us too. When we need to xfail a single scenario or give it a different fixture, today that means dropping scenarios() and hand-writing every @scenario in that file. A subclass hook would keep the one-liner for the common case and only cost you on the scenario you actually change.

Happy to try the class-return approach against our suite and report back what breaks.

[youtux]

yeah I really don't have much time for this or even the drive to do it, as I'm not using pytest-bdd in my current gig

[youtux]

that being said, if you are willing to work on it (or spend AI tokens on it), I'll try to put some time to review and give directions

[golikovichev]

Thanks, that works for me. I'll take a run at it and keep you posted.

Before I write much code, one design fork I'd like your steer on: backward compatibility.

Today scenarios("x.feature") injects the test functions into the module globals, so they get collected without the caller binding anything. A class-return changes that contract: TestX = scenarios("x.feature") is only collected if it's bound to a Test-prefixed name. So two options:

1. Return the class and keep the implicit injection, so existing files keep working and the return value is opt-in for people who want the subclass override. Lowest friction, but scenarios() then does two things.
2. Return the class only and treat injection as deprecated. Cleaner long term, but it is a breaking change and needs a deprecation cycle.

I lean towards 1 for a first pass (additive, no break), with 2 as a follow-up if you want to retire injection later. Does that match how you'd want it to land?

Smaller things I'll pin down as I go: how method names are derived from scenario names (snake_case, duplicate or colliding names), and how this plays with scenario outlines and parametrized examples. I'll start from your stub and open a draft PR early so you can redirect before I go too far.

[youtux]

Yeah ideally we should make a release where an "old" codebase would still work, but we emit warnings so that downstream users can fix their code while keeping the tests working.
Then in a next major release we stop emitting warnings and stop injecting into the caller module altogether.

I'm also ok with creating a new api rather than reusing the old "scenarios" decorator.
So that in the second version (the breaking change one), we can just remove the scenarios function, and it would fail loudly for downstream users that didn't update their code.

Actually, I do prefer this second way.

[golikovichev]

Makes sense, that is the cleaner path. So the plan: a new API for the class-returning form (its own name, not overloading scenarios), scenarios() kept working but emitting a DeprecationWarning that points at it, then the next major drops the injection so it fails loudly.

Any preference on the name? I was thinking feature_tests(...) or scenarios_class(...), whatever reads best to you. I will open a draft PR with the new API plus the deprecation warning so you can redirect early.