Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Invocation scoped fixtures #1711

Merged
merged 11 commits into from
Jul 21, 2016
Merged

Invocation scoped fixtures #1711

merged 11 commits into from
Jul 21, 2016

Conversation

nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Jul 9, 2016 •
edited
Loading

Implementation for invocation-scoped fixtures as we discussed during the sprint. There are some things still missing on the PR, but I would like to get feedback from what's implemented so far.

@hpk42 I would appreciate your feedback, as I'm sure you are the most knowledgeable person on the fixture machinery. 😁

TODO:

  • Add some more isolated tests
  • Update CHANGELOG
  • Some code cleanups, noted below

@nicoddemus nicoddemus added this to the 3.0 milestone Jul 9, 2016
nicoddemus
nicoddemus reviewed Jul 9, 2016
View reviewed changes
_pytest/capture.py
@@ -172,7 +172,7 @@ def capfd(request):
captured output available via ``capfd.readouterr()`` method calls
which return a ``(out, err)`` tuple.
"""
if "capsys" in request._funcargs:
if "capsys" in request.fixturenames:
Copy link
Member Author

@nicoddemus nicoddemus Jul 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

note to self: add a test here that this will work when capsys/capfd are requested using request.getfixturevalue()

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@coveralls
Copy link

coveralls commented Jul 9, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 92.159% when pulling 94448a3 on nicoddemus:invocation-scoped-fixtures into 7a2058e on pytest-dev:features.

@nicoddemus nicoddemus force-pushed the invocation-scoped-fixtures branch from 94448a3 to a001445 Compare July 9, 2016 20:08
@coveralls
Copy link

coveralls commented Jul 9, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 92.159% when pulling a001445 on nicoddemus:invocation-scoped-fixtures into 7a2058e on pytest-dev:features.

@nicoddemus nicoddemus force-pushed the invocation-scoped-fixtures branch from a001445 to 8ed055e Compare July 10, 2016 01:53
@coveralls
Copy link

coveralls commented Jul 10, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 92.159% when pulling 8ed055e on nicoddemus:invocation-scoped-fixtures into 7a2058e on pytest-dev:features.

@coveralls
Copy link

coveralls commented Jul 11, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 92.172% when pulling 0ca0696 on nicoddemus:invocation-scoped-fixtures into 6f68dfc on pytest-dev:features.

@nicoddemus nicoddemus force-pushed the invocation-scoped-fixtures branch from e206937 to 0dd1c8b Compare July 11, 2016 23:48
@coveralls
Copy link

coveralls commented Jul 11, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 92.172% when pulling e206937 on nicoddemus:invocation-scoped-fixtures into 6f68dfc on pytest-dev:features.

@coveralls
Copy link

coveralls commented Jul 11, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 92.172% when pulling 0dd1c8b on nicoddemus:invocation-scoped-fixtures into 6f68dfc on pytest-dev:features.

@nicoddemus
Copy link
Member Author

I think this is ready for reviewing

@nicoddemus
Copy link
Member Author

FWIW I changed (locally) tmpdir and monkeypatch to invocation, and the test suite ran fine. 😁

hpk42
hpk42 reviewed Jul 15, 2016
View reviewed changes
CHANGELOG.rst
any scope, when they assume the same scope as the fixture requesting it. An ``invocation``-scoped
fixture can be requested from different scopes in the same test session,
in which case each scope will have its own copy. This feature is considered experimental.
Thanks `@nicoddemus`_ for the PR.
Copy link
Contributor

Choose a reason for hiding this comment

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

i'd rather write:

Experimentally introduce new "invocation" fixture scope. At invocation scope a fixture function is cached in the same way as the fixture or test function that requests it. You can now use the builtin monkeypatch fixture from session-scoped fixtures where previously you would get an error that you can not use a function-scoped fixture from a session scoped one.

@hpk42
Copy link
Contributor

hpk42 commented Jul 15, 2016

all looks good. I see you implemented the ":SCOPE" naming idea we discussed after the sprint and it seems to work, great! So just a few more fine grained tests missing and it's ready to merge. thanks for the good work!

@bubenkoff
Copy link
Member

@nicoddemus great job

@nicoddemus
Copy link
Member Author

Applied the requested changes, I'll merge this myself tomorrow unless someone else does it first or has more comments. 😄

@coveralls
Copy link

coveralls commented Jul 21, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 93.048% when pulling 05f3422 on nicoddemus:invocation-scoped-fixtures into eaa4ee3 on pytest-dev:features.

@nicoddemus nicoddemus merged commit ae07985 into pytest-dev:features Jul 21, 2016
@nicoddemus nicoddemus deleted the invocation-scoped-fixtures branch July 21, 2016 22:48
@nicoddemus nicoddemus mentioned this pull request Dec 28, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.0
Development

Successfully merging this pull request may close these issues.
4 participants
@nicoddemus @coveralls @hpk42 @bubenkoff