Skip to content

Improve error handling around yieldctx fixtures which do not yield a value#7083

Merged
bluetech merged 4 commits intopytest-dev:masterfrom
symonk:7061-improvement-fixtures-without-yield-value
Apr 15, 2020
Merged

Improve error handling around yieldctx fixtures which do not yield a value#7083
bluetech merged 4 commits intopytest-dev:masterfrom
symonk:7061-improvement-fixtures-without-yield-value

Conversation

@symonk
Copy link
Copy Markdown
Member

@symonk symonk commented Apr 13, 2020 •
edited by bluetech
Loading

Fixes #7061.

Good afternoon, thanks for your time. I am opening this PR as a 'work in progress'. I have implemented some behaviour that checks a yielding fixture actually yields a value with a test to cover it.

What I am not so sure on is what to do exactly when that occurs; right now I am raising a ValueError, not entirely sure that is the way to go and secondly what exactly pytest should do in that instance. Fixture(s) are so core to pytest so being so new to the code makes me a little nervous. Regression tests are passing atleast...

  • .rst in changelog has been created
  • Allow maintainers to push and squash when merging my commits. Please uncheck this if you

bluetech
bluetech requested changes Apr 14, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

Thanks @symonk, I left some comments.

I think ValueError is fine, at least it seems to be used in other similar situation, from what I can see.

I added a Fixes #7061 to the PR description so that it will auto-close it when merged.

@symonk
Copy link
Copy Markdown
Member Author

symonk commented Apr 14, 2020

@bluetech appreciate your feed back once again, will do this after work this evening

@symonk
Copy link
Copy Markdown
Member Author

symonk commented Apr 14, 2020 •
edited
Loading

not sure where to go on these (ubuntu-pyp3):

FAILED testing/python/raises.py::TestRaises::test_raises_cyclic_reference[function_match]
FAILED testing/python/raises.py::TestRaises::test_raises_cyclic_reference[with]
= 2 failed, 2582 passed, 88 skipped, 9 xfailed, 30 warnings in 223.63s (0:03:43) =
ERROR: InvocationError for command /home/runner/work/pytest/pytest/.tox/pypy3-xdist/bin/pytest -n auto (exited with code 1)

@bluetech
Copy link
Copy Markdown
Member

bluetech commented Apr 14, 2020

not sure where to go on these (ubuntu-pyp3)

Seems unrelated. Some tests are flaky...

@symonk
Copy link
Copy Markdown
Member Author

symonk commented Apr 14, 2020

Thanks for your time @bluetech, I really appreciate your patience and I apologize if I am testing it with many discussions and tweaks/changes. This should be completed now (I hope)

@symonk symonk requested a review from bluetech April 14, 2020 16:03
@symonk symonk marked this pull request as ready for review April 14, 2020 16:07
@symonk symonk changed the title initial work for yielding fixtures failing to yield a value Improve error handling around yieldctx fixtures which do not yield a value Apr 14, 2020
@bluetech bluetech merged commit de6c28e into pytest-dev:master Apr 15, 2020
@symonk symonk deleted the 7061-improvement-fixtures-without-yield-value branch May 22, 2020 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bluetech bluetech bluetech approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fixtures with yield statement in if-else clause crash pytest

2 participants

@symonk @bluetech