Skip to content

Don't skip fixtures that are substrings of params #926

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

Merged
merged 3 commits into from
Aug 11, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ Daniel Grana
Daniel Nuri
Dave Hunt
David Mohr
Edison Gustavo Muenz
Eduardo Schettino
Eric Hunsberger
Eric Siegerman
Florian Bruhin
Edison Gustavo Muenz
Eric Hunsberger
Floris Bruynooghe
Graham Horler
Grig Gheorghiu
Expand All @@ -47,6 +47,7 @@ Maciek Fijalkowski
Maho
Marc Schlaich
Mark Abramowitz
Markus Unterwaditzer
Martijn Faassen
Nicolas Delaby
Pieter Mulder
Expand Down
8 changes: 6 additions & 2 deletions CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,18 @@

- Fix #562: @nose.tools.istest now fully respected.

- Fix issue736: Fix a bug where fixture params would be discarded when combined
with parametrization markers.
Thanks to Markus Unterwaditzer for the PR.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you forgot to add yourself to AUTHORS.


- parametrize now also generates meaningful test IDs for enum, regex and class
objects (as opposed to class instances).
Thanks to Florian Bruhin for the PR.

- Add 'warns' to assert that warnings are thrown (like 'raises').
Thanks to Eric Hunsberger for the PR.

- Fix #683: Do not apply an already applied mark. Thanks ojake for the PR.
- Fix issue683: Do not apply an already applied mark. Thanks ojake for the PR.

- Deal with capturing failures better so fewer exceptions get lost to
/dev/null. Thanks David Szotten for the PR.
Expand All @@ -36,7 +40,7 @@
deprecated.
Thanks Bruno Oliveira for the PR.

- fix issue 808: pytest's internal assertion rewrite hook now implements the
- fix issue808: pytest's internal assertion rewrite hook now implements the
optional PEP302 get_data API so tests can access data files next to them.
Thanks xmo-odoo for request and example and Bruno Oliveira for
the PR.
Expand Down
2 changes: 1 addition & 1 deletion _pytest/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -1818,7 +1818,7 @@ def pytest_generate_tests(self, metafunc):
if fixturedef.params is not None:
func_params = getattr(getattr(metafunc.function, 'parametrize', None), 'args', [[None]])
# skip directly parametrized arguments
if argname not in func_params and argname not in func_params[0]:
if argname not in func_params:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't figure out why this line is needed. It has been introduced with https://bitbucket.org/pytest-dev/pytest/pull-requests/257/allow-to-override-parametrized-fixtures

I suppose it's due to strings like value,othervalue, but it doesn't seem like it's actually needed (see new python/collect.py-testcases). In other words, no item of func_params ever contains a comma.

metafunc.parametrize(argname, fixturedef.params,
indirect=True, scope=fixturedef.scope,
ids=fixturedef.ids)
Expand Down
12 changes: 11 additions & 1 deletion testing/python/collect.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,9 +412,19 @@ def value():
['overridden'])
def test_overridden_via_param(value):
assert value == 'overridden'

@pytest.mark.parametrize('somevalue', ['overridden'])
def test_not_overridden(value, somevalue):
assert value == 'value'
assert somevalue == 'overridden'

@pytest.mark.parametrize('other,value', [('foo', 'overridden')])
def test_overridden_via_multiparam(other, value):
assert other == 'foo'
assert value == 'overridden'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for completeness, can you add a assert other == 'foo' here?

""")
rec = testdir.inline_run()
rec.assertoutcome(passed=1)
rec.assertoutcome(passed=3)


def test_parametrize_overrides_parametrized_fixture(self, testdir):
Expand Down
16 changes: 16 additions & 0 deletions testing/python/fixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -1598,6 +1598,22 @@ def test_result():
reprec = testdir.inline_run()
reprec.assertoutcome(passed=4)

def test_multiple_parametrization_issue_736(self, testdir):
testdir.makepyfile("""
import pytest

@pytest.fixture(params=[1,2,3])
def foo(request):
return request.param

@pytest.mark.parametrize('foobar', [4,5,6])
def test_issue(foo, foobar):
assert foo in [1,2,3]
assert foobar in [4,5,6]
""")
reprec = testdir.inline_run()
reprec.assertoutcome(passed=9)

def test_scope_session(self, testdir):
testdir.makepyfile("""
import pytest
Expand Down