Skip to content

capture: cleanup item fixture handling #6663

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
Feb 7, 2020

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Feb 3, 2020

This started by looking at how to get the current test item in general,
and then I noticed that it is not necessary for the capture plugin to
track it manually in the first place.

Changes the API for CaptureManager.*_fixture methods (does not take item
anymore). This could be kept / ignored if required.

This started by looking at how to get the current test item in general,
and then I noticed that it is not necessary for the capture plugin to
track it manually in the first place.
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

AFAIK this is practically an internal api, so the change is fine

@blueyed
Copy link
Contributor Author

blueyed commented Feb 3, 2020

Thanks for the review. The functions are at least not in the docs apparently, as CaptureManager itself (I've thought capman would be considered public)

Would like to get @nicoddemus's opinion also, given that I've seen him doing work in that area.
Initially I've thought it would be necessary to have them on the item (e.g. for later use or similar).

@blueyed blueyed requested a review from nicoddemus February 4, 2020 02:09
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Thanks! Indeed this looks better.

Please consider my naming suggestions though. 👍

@blueyed
Copy link
Contributor Author

blueyed commented Feb 7, 2020

Pushed a fixup with 2/3 suggestions applied.

@blueyed blueyed merged commit b4ace46 into pytest-dev:features Feb 7, 2020
@blueyed blueyed deleted the capture-cleanup branch February 7, 2020 18:23
blueyed added a commit to blueyed/pytest that referenced this pull request Feb 9, 2020
Based on the removed doc for `_install_capture_fixture_on_item`.

Follow-up to pytest-dev#6663.
blueyed added a commit to blueyed/pytest that referenced this pull request Feb 12, 2020
Based on the removed doc for `_install_capture_fixture_on_item`.

Follow-up to pytest-dev#6663.

Update src/_pytest/capture.py  [ci skip]

Co-Authored-By: Ran Benita <[email protected]>
blueyed added a commit that referenced this pull request Feb 13, 2020
Based on the removed doc for `_install_capture_fixture_on_item`.

Follow-up to #6663.

Co-authored-by: Ran Benita <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants