Repository: pytest-dev/pytest
Pull request: Improve error handling around yieldctx fixtures which do not yield a value
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...

- [X] .rst in changelog has been created
- [X] Allow maintainers to push and squash when merging my commits. Please uncheck this if you 

Problem statement:
fixtures with yield statement in if-else clause crash pytest

If I have a fixture with a yield statement within an else clause and the else clause is False, pytest will crash

Here is a simple example (note one of the parametrize lines is purposely commented out as you can use it to toggle back and forth.  Only when "filename" is None does it crash.  Otherwise it runs fine.  Am I missing something?  Is there a different way that I'm suppose to do this?  My objective is to only have the fixture do something if the filename passed to the fixture is not None.  If there is a valid filename, then I want the fixture to do pre-setup work and then yield and then post test cleanup....

```
import pytest

#################################################
# Simple test
#################################################
#@pytest.mark.parametrize("dut, filename", [("dev1", "myfile.yml"),])
@pytest.mark.parametrize("dut, filename", [("dev1", None),])
def test_1(dut, filename, preconfig_fix2):
    print("Running test function")

@pytest.fixture(scope="function")
def preconfig_fix2(request,):

    filename = request.getfuncargvalue("filename")
    if filename is None:
        print("There is nothing to do here!!")
    else:
        print("There is work to do")
        yield
        print("Finishing up")
```

python --version
Python 3.7.4

Ubuntu 16.04.6 LTS

pip freeze
alerta==6.5.0
allure-pytest==2.6.5
allure-python-commons==2.6.5
ansi2html==1.5.2
asn1crypto==0.24.0
astroid==2.2.5
atomicwrites==1.3.0
attrs==19.1.0
auto-reporting-system==1.3.3
auto-ssh-client==1.0
auto-tests-fw==1.0
auto-utils==1.0
backcall==0.1.0
backports.ssl==0.0.9
bcrypt==3.1.7
bitarray==1.0.1
blackswan==1.0.75
bleach==3.1.0
certifi==2019.9.11
cffi==1.12.3
chardet==3.0.4
Click==7.0
colorlog==4.0.2
cryptography==2.7
dateparser==0.7.2
datetime-extractor==0.5
decorator==4.1.2
defusedxml==0.6.0
dill==0.3.0
dn-grpc-client==1.0.0
dn-ncclient==1.0.0
docker==3.7.0
docker-pycreds==0.4.0
docutils==0.15.2
dominate==2.3.5
easysnmp==0.2.5
elasticsearch==6.1.1
elasticsearch-dsl==6.1.0
entrypoints==0.3
enum34==1.1.6
Flask==1.0.2
Flask-Bootstrap==3.3.7.1
Flask-Login==0.4.1
flask-nav==0.6
Flask-WTF==0.14.2
future==0.17.1
gevent==1.4.0
GitHub-Flask==3.2.0
googleapis-common-protos==1.5.8
greenlet==0.4.15
grpcio==1.19.0
idna==2.6
importlib-metadata==0.19
ipaddress==1.0.22
ipykernel==5.1.2
ipython==7.8.0
ipython-genutils==0.2.0
ipywidgets==7.5.1
isort==4.3.4
itsdangerous==1.1.0
ixia==1.0.0
IxNetwork==8.50.1501.10
ixnetwork-restpy==1.0.51
jedi==0.15.1
Jinja2==2.10
jira==2.0.0
jsonschema==3.0.2
jupyter==1.0.0
jupyter-client==5.3.3
jupyter-console==6.0.0
jupyter-core==4.5.0
lazy-object-proxy==1.4.2
libraries==1.0
Logbook==1.5.2
lxml==4.1.1
MarkupSafe==1.1.1
mccabe==0.6.1
mistune==0.8.4
more-itertools==7.2.0
natsort==6.0.0
nbconvert==5.6.0
nbformat==4.4.0
ncclient==0.6.4
netaddr==0.7.19
netmiko==2.4.1
notebook==6.0.1
numpy==1.17.2
oauthlib==3.1.0
packaging==19.1
pandas==0.25.1
pandocfilters==1.4.2
paramiko==2.5.0
parso==0.5.1
pbr==5.4.3
pexpect==4.7.0
pickleshare==0.7.5
pluggy==0.12.0
ply==3.11
prometheus-client==0.7.1
prompt-toolkit==2.0.9
protobuf==3.9.1
psycopg2-binary==2.8
ptyprocess==0.6.0
py==1.8.0
pyaml==19.4.1
pyang==2.0.1
pyangbind==0.8.1
pyasn1==0.4.7
pycodestyle==2.5.0
pycparser==2.19
pycryptodomex==3.9.0
pydocstyle==4.0.1
pyflakes==2.1.1
Pygments==2.4.2
pygrok==1.0.0
PyJWT==1.7.1
pylama==7.6.6
pylint==2.3.1
PyNaCl==1.3.0
pyOpenSSL==19.0.0
pyparsing==2.4.2
pyrsistent==0.15.4
pyserial==3.4
pyshark==0.4.2.5
pysmi==0.3.4
pysnmp==4.4.6
pytest==3.7.0
pytest-check==0.3.5
pytest-csv==2.0.1
pytest-dependency==0.4.0
pytest-instafail==0.4.0
pytest-ordering==0.6
pytest-repeat==0.7.0
pytest-reportportal==1.0.5
python-consul==1.1.0
python-dateutil==2.6.1
pytz==2017.3
PyXB==1.2.6
PyYAML==5.1.1
pyzmq==18.1.0
qtconsole==4.5.5
redis==2.10.6
regex==2019.6.8
rejson==0.2.1
reportportal-client==3.2.0
requests==2.18.4
requests-oauthlib==1.2.0
requests-toolbelt==0.9.1
scp==0.13.2
selenium==3.141.0
semantic-version==2.6.0
Send2Trash==1.5.0
six==1.12.0
snowballstemmer==1.9.1
SQLAlchemy==1.3.2
tabulate==0.8.2
terminado==0.8.2
testpath==0.4.2
textfsm==1.1.0
timeout-decorator==0.4.1
tornado==6.0.3
traitlets==4.3.2
typed-ast==1.4.0
tzlocal==2.0.0
urllib3==1.22
visitor==0.1.3
waiting==1.4.1
wcwidth==0.1.7
webencodings==0.5.1
websocket-client==0.56.0
Werkzeug==0.15.6
widgetsnbextension==3.5.1
wrapt==1.11.2
WTForms==2.2.1
WTForms-JSON==0.3.3
XlsxWriter==1.0.2
xmltodict==0.11.0
yangson==1.3.20
zipp==0.5.2

Hints from issue discussion:
Can you show the output you get?
Here is the ouput.  I removed the conftest.py file so nothing else should be running....

```
(p1_netmiko_exper) as2863@ubuntuP1-SYSlog-S1:~/pythonProjects/p1-automation$ pytest lag/test_yield.py  -s
============================================================================================================ test session starts ============================================================================================================
platform linux -- Python 3.7.4, pytest-3.7.0, py-1.8.0, pluggy-0.12.0
rootdir: /home/as2863/pythonProjects/p1-automation, inifile: pytest.ini
plugins: csv-2.0.1, allure-pytest-2.6.5, check-0.3.5, pylama-7.6.6, reportportal-1.0.5, dependency-0.4.0, instafail-0.4.0, auto-reporting-system-1.3.3, ordering-0.6, repeat-0.7.0
collected 1 item

lag/test_yield.py There is nothing to do here!!
E

================================================================================================================== ERRORS ===================================================================================================================
____________________________________________________________________________________________________ ERROR at setup of test_1[dev1-None] ____________________________________________________________________________________________________

self = <CallInfo when='setup' exception: >, func = <function call_runtest_hook.<locals>.<lambda> at 0x7f3c96c4a050>, when = 'setup', treat_keyboard_interrupt_as_exception = False

    def __init__(self, func, when, treat_keyboard_interrupt_as_exception=False):
        #: context of invocation: one of "setup", "call",
        #: "teardown", "memocollect"
        self.when = when
        self.start = time()
        try:
>           self.result = func()

../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/_pytest/runner.py:201:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/_pytest/runner.py:183: in <lambda>
    lambda: ihook(item=item, **kwds),
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/pluggy/hooks.py:289: in __call__
    return self._hookexec(self, self.get_hookimpls(), kwargs)
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/pluggy/manager.py:87: in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/pluggy/manager.py:81: in <lambda>
    firstresult=hook.spec.opts.get("firstresult") if hook.spec else False,
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/_pytest/runner.py:104: in pytest_runtest_setup
    item.session._setupstate.prepare(item)
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/_pytest/runner.py:370: in prepare
    col.setup()
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/_pytest/python.py:1435: in setup
    fixtures.fillfixtures(self)
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/_pytest/fixtures.py:293: in fillfixtures
    request._fillfixtures()
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/_pytest/fixtures.py:464: in _fillfixtures
    item.funcargs[argname] = self.getfixturevalue(argname)
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/_pytest/fixtures.py:508: in getfixturevalue
    return self._get_active_fixturedef(argname).cached_result[0]
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/_pytest/fixtures.py:531: in _get_active_fixturedef
    self._compute_fixture_value(fixturedef)
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/_pytest/fixtures.py:602: in _compute_fixture_value
    fixturedef.execute(request=subrequest)
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/_pytest/fixtures.py:895: in execute
    return hook.pytest_fixture_setup(fixturedef=self, request=request)
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/pluggy/hooks.py:289: in __call__
    return self._hookexec(self, self.get_hookimpls(), kwargs)
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/pluggy/manager.py:87: in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/pluggy/manager.py:81: in <lambda>
    firstresult=hook.spec.opts.get("firstresult") if hook.spec else False,
../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/_pytest/fixtures.py:944: in pytest_fixture_setup
    result = call_fixture_func(fixturefunc, request, kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

fixturefunc = <function preconfig_fix2 at 0x7f3c98a04b90>, request = <SubRequest 'preconfig_fix2' for <Function 'test_1[dev1-None]'>>
kwargs = {'__being_called_by_pytest': True, 'request': <SubRequest 'preconfig_fix2' for <Function 'test_1[dev1-None]'>>}

    def call_fixture_func(fixturefunc, request, kwargs):
        yieldctx = is_generator(fixturefunc)
        if yieldctx:
            it = fixturefunc(**kwargs)
>           res = next(it)
E           StopIteration

../../python-venv/p1_netmiko_exper/lib/python3.7/site-packages/_pytest/fixtures.py:794: StopIteration
========================================================================================================== 1 error in 0.18 seconds ==========================================================================================================
(p1_netmiko_exper) as2863@ubuntuP1-SYSlog-S1:~/pythonProjects/p1-automation$
```
It looks like pytest falsely assumes that it can call `next(...)` at least once if a fixture is a generator. Here's a simpler example:

```python
import pytest

@pytest.fixture
def fixt():
    if False:
        yield

def test_fixt(fixt):
    pass
```

Not sure what the best behavior would be in that case, though?

- Show a warning and run the test (with `None` as the fixture value)
- Make it an error
- Just run the test

In your case, you might want to add a `yield` inside of the `if` part as well.
a `yield fixture without yield` should trigger a indicating error that the fixture produced no value
with a mention that exactly one yield is expected
@The-Compiler thanks for the compact reducer :+1: 
So not sure if I'm clear... Should what I'm trying to do work?  or not?  Are you thinking this is a bug at this point?  
Thx.
@allan-silverstein - it's not meant to work, but we want to give a more helpful error in future :smile:  

For your specific case, I'd try:

```python
if filename is None:
    pytest.skip("There is nothing to do here!!")
```
Hmm, that does not solve my issue though.  If I use pytest.skip(), then the test function itself is skipped.  I still need the test function to run.  Again, if filename is None, I just want the fixture to return back to the test function... Actually, I'm not really following why the "yield" statement causes the problem because the yield statement should only be processed when filename is "not" None... Probably my lac of understanding of the internals... 

Any other suggestions to get this to work?
Thx
you *NEED* the yield statement because that's how the yield fixture communicates **setup done**
so if you have it in a if/else, you certainly need it on both sides of the if/else
Ahhh, putting the yield statement on both sides of the if/else is an easy solution.
Thank you.

Commit message: initial work for yielding fixtures failing to yield a value

Patch to review:
diff --git a/changelog/7061.improvement.rst b/changelog/7061.improvement.rst
new file mode 100644
index 00000000000..7c12d408340
--- /dev/null
+++ b/changelog/7061.improvement.rst
@@ -0,0 +1 @@
+Raise a ValueError when a yielding fixture fails to yield a single value
diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py
index f9e0f3b28b5..f6d4d03a690 100644
--- a/src/_pytest/fixtures.py
+++ b/src/_pytest/fixtures.py
@@ -786,13 +786,21 @@ def fail_fixturefunc(fixturefunc, msg):
 def call_fixture_func(fixturefunc, request, kwargs):
     yieldctx = is_generator(fixturefunc)
     if yieldctx:
-        it = fixturefunc(**kwargs)
-        res = next(it)
-        finalizer = functools.partial(_teardown_yield_fixture, fixturefunc, it)
-        request.addfinalizer(finalizer)
+        generator = fixturefunc(**kwargs)
+        try:
+            fixture_return_value = next(generator)
+        except StopIteration:
+            raise ValueError(
+                "Fixture {} did not yield a value".format(fixturefunc.__name__)
+            )
+        else:
+            finalizer = functools.partial(
+                _teardown_yield_fixture, fixturefunc, generator
+            )
+            request.addfinalizer(finalizer)
     else:
-        res = fixturefunc(**kwargs)
-    return res
+        fixture_return_value = fixturefunc(**kwargs)
+    return fixture_return_value
 
 
 def _teardown_yield_fixture(fixturefunc, it):
diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py
index 36e55a0e1de..eb0dd4cc35d 100644
--- a/testing/python/fixtures.py
+++ b/testing/python/fixtures.py
@@ -3,6 +3,7 @@
 
 import pytest
 from _pytest import fixtures
+from _pytest.config import ExitCode
 from _pytest.fixtures import FixtureRequest
 from _pytest.pathlib import Path
 from _pytest.pytester import get_public_names
@@ -4290,3 +4291,23 @@ def test_suffix(fix_combined):
     )
     result = testdir.runpytest("-vv", str(p1))
     assert result.ret == 0
+
+
+def test_yield_fixture_with_no_value(testdir):
+    testdir.makepyfile(
+        """
+        import pytest
+        @pytest.fixture
+        def empty_yield():
+            if False:
+                yield
+
+        def test_fixt(empty_yield):
+            pass
+        """
+    )
+    expected = "E               ValueError: Fixture empty_yield did not yield a value"
+    result = testdir.runpytest()
+    result.assert_outcomes(error=1)
+    result.stdout.fnmatch_lines([expected])
+    assert result.ret == ExitCode.TESTS_FAILED


Code review comments:
diff_hunk: @@ -0,0 +1 @@
+Raise a ValueError when a yielding fixture fails to yield a single value
path: changelog/7061.improvement.rst
text: @user1:
```suggestion
When a yielding fixture fails to yield a value, report a test setup error instead of crashing.
```

diff_hunk: @@ -786,13 +786,21 @@ def fail_fixturefunc(fixturefunc, msg):
 def call_fixture_func(fixturefunc, request, kwargs):
     yieldctx = is_generator(fixturefunc)
     if yieldctx:
-        it = fixturefunc(**kwargs)
-        res = next(it)
-        finalizer = functools.partial(_teardown_yield_fixture, fixturefunc, it)
-        request.addfinalizer(finalizer)
+        generator = fixturefunc(**kwargs)
+        try:
+            fixture_return_value = next(generator)
+        except StopIteration:
+            raise ValueError(
+                "Fixture {} did not yield a value".format(fixturefunc.__name__)
+            )
path: src/_pytest/fixtures.py
text: @user1:
I don't think the cause is interesting in this case, and just makes the traceback less tidy, so I'd omit it:

```suggestion
            ) from None
```

@author:
are we sure we don't want to say something? the 7061 issue suggests we should explain the reason to them, this might still be a little vague? perhaps:

`Yielding fixture failed to yield a value` ?

@user1:
I think "Yielding" in "Yielding fixture" is redundant because the continuation is "failed to yield ...".

I also liked that in the previous version the the fixture name was provided in the error message, to pinpoint the issue. BTW, if you bring it back, should probably use `request.fixturename` (the fixture name, can be set explicitly with `@user2.fixture(name='...')) rather than the name of the fixture function itself.

Finally, I prefer "did not yield a value" over "did not yield a single value", because the latter implies it either didn't yield or yielded more than once, but we don't check for more than once.

@author:
Hey great feedback thanks! i was wondering how to get the name easier when name='' was provided, great idea! could we agree on the following error message:

`Fixture: {request.fixturename} did not yield a value` ? I will defer to your expertise and implement

@user1:
The ":" makes it read like not a proper sentence, so I prefer without it, unless there is some reason to have it?

@author:
nope not at all, a bad habbit of mine :) will implement w/o

diff_hunk: @@ -786,13 +786,21 @@ def fail_fixturefunc(fixturefunc, msg):
 def call_fixture_func(fixturefunc, request, kwargs):
     yieldctx = is_generator(fixturefunc)
     if yieldctx:
-        it = fixturefunc(**kwargs)
-        res = next(it)
-        finalizer = functools.partial(_teardown_yield_fixture, fixturefunc, it)
-        request.addfinalizer(finalizer)
+        generator = fixturefunc(**kwargs)
+        try:
+            fixture_return_value = next(generator)
+        except StopIteration:
+            raise ValueError(
+                "Fixture {} did not yield a value".format(fixturefunc.__name__)
+            )
+        else:
+            finalizer = functools.partial(
+                _teardown_yield_fixture, fixturefunc, generator
+            )
+            request.addfinalizer(finalizer)
path: src/_pytest/fixtures.py
text: @user1:
To reduce indentation, would omit the `else` since it's not really needed:

```suggestion
        finalizer = functools.partial(_teardown_yield_fixture, fixturefunc, generator)
        request.addfinalizer(finalizer)
```

diff_hunk: @@ -786,13 +786,21 @@ def fail_fixturefunc(fixturefunc, msg):
 def call_fixture_func(fixturefunc, request, kwargs):
     yieldctx = is_generator(fixturefunc)
     if yieldctx:
-        it = fixturefunc(**kwargs)
-        res = next(it)
-        finalizer = functools.partial(_teardown_yield_fixture, fixturefunc, it)
-        request.addfinalizer(finalizer)
+        generator = fixturefunc(**kwargs)
+        try:
+            fixture_return_value = next(generator)
path: src/_pytest/fixtures.py
text: @user1:
"fixture return value" might be a bit confusing in the generator case, because a generator does actually have a return value, which is different from the yielded value(s). My suggestion is `fixture_result`.