Fixes #276: configure and unconfigure Feature class #282
Conversation
… tests separation. Do not use deprecated pytest.config global variable
|
Can we firstly merge this pull requests as it fixes CI tests? Please review :) |
pytest_bdd/plugin.py
Outdated
| @@ -47,14 +49,19 @@ def add_bdd_ini(parser): | |||
| @pytest.mark.trylast | |||
| def pytest_configure(config): | |||
| """Configure all subplugins.""" | |||
| ConfigStack.add(config) | |||
There was a problem hiding this comment.
Why do you need a stack at all? Shouldn't the hook only be called once?
There was a problem hiding this comment.
It is all here to fix tests that we run using testdir plugin.
Our problem is that:
- When we start all tests it calls
pytest_configure. - Then when it starts some test with
testdirit callspytest_configureagain with new config. - After test (as in current code there is no
pytest_unconfigure) it just runs another test. - As
pytest_configurefrom point 2 changed pytest config - our tests start to fail as they expect default config (from point 1).
I added pytest_unconfigure hook so now after running test using testdir it can bring back previously used config.
So this pull request changes logic to:
- When we start all tests it calls
pytest_configure(we store config on stack). - Then when it starts some test with
testdirit callspytest_configureagain with new config (again we store new config on stack). - After tests it calls
pytest_unconfigure-> we bring back previous config from the stack. - We have previous config back and tests pass :)
Please note that it has been discussed also here pytest-dev/pytest#4495
pytest_bdd/utils.py
Outdated
| def get_previous(cls): | ||
| cls._stack.pop() | ||
| if cls._stack: | ||
| return cls._stack[-1] |
There was a problem hiding this comment.
There should probably be an explicit return None here.
pytest_bdd/utils.py
Outdated
|
|
||
| @classmethod | ||
| def get_pytest_bdd_apply_tag_hook(cls): | ||
| return cls._stack[-1].hook.pytest_bdd_apply_tag |
There was a problem hiding this comment.
Might be cleaner to make this method call the hook directly (by taking *args/**kwargs).
… None at the end of the ConfigStack get_previous method
In tests use -m instead of -k for running tests by marker expressions
|
Merged #287 |
|
This pull request fixes CI tests |
|
Would be great if someone else could take a look at this as well - I'm quite busy with exams coming up, so I don't trust my brain enough to review bigger things right now 😉 |
There was a problem hiding this comment.
Sorry if it took me so long to review this case, @sliwinski-milosz
pytest_bdd/utils.py
Outdated
| @@ -110,3 +110,23 @@ def get_markers_args_using_iter_markers(node, mark_name): | |||
| def get_markers_args_using_get_marker(node, mark_name): | |||
| """Deprecated on pytest>=3.6""" | |||
| return getattr(node.get_marker(mark_name), 'args', ()) | |||
|
|
|||
|
|
|||
| class ConfigStack(object): | |||
There was a problem hiding this comment.
Do we really need to define an object to handle a stack? I'd rather use just a list and name it config_stack
There was a problem hiding this comment.
Changed to list
pytest_bdd/utils.py
Outdated
| cls._stack.append(config) | ||
|
|
||
| @classmethod | ||
| def get_previous(cls): |
There was a problem hiding this comment.
also because get_previous should not have side effects, but instead it removes the current element and return the previous one
| @@ -250,6 +250,8 @@ def __bool__(self): | |||
| class Feature(object): | |||
|
|
|||
| """Feature.""" | |||
| strict_gherkin = None | |||
There was a problem hiding this comment.
I don't think that the Feature class should contain configuration options like strict_gherkin or base_dir. Instead, callers of Feature(...) should consult the current config from the stack.
|
Closing in favour of: |
To be able to unconfigure Feature class during test execution we need to access previous config. I added
ConfigStackfor that.I used that stack also to avoid accessing deprecated
pytest.configwhile gettingconfig.hook.pytest_bdd_apply_tag.I also think that
def scenarios(*feature_paths, **kwargs):needs refactor.