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.

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

Support pytest plugins that modify the fixture closure list #176

Closed
HMaker opened this issue Jan 7, 2021 · 11 comments
Closed

Support pytest plugins that modify the fixture closure list #176

HMaker opened this issue Jan 7, 2021 · 11 comments

Comments

@HMaker
Copy link

HMaker commented Jan 7, 2021

I tried to parametrize a test function marked by pytest-asyncio

import random
import pytest
from pytest_cases import parametrize_with_cases


def sum(a, b):
    return a + b

@pytest.fixture
def random_num():
    return random.randint(0, 65543)

def case_1(random_num: int):
    return 1, random_num, 1 + random_num

def case_2(random_num: int):
    return 2, random_num, 2 + random_num

@pytest.mark.asyncio
@parametrize_with_cases('case', cases='.')
async def test_sum_of(case):
    a, b, c = case
    assert sum(a, b) == c

but pytest_cases raises NotImplementedError: When fixture unions are present, inserting a fixture in the closure is non-trivial. Please report this so that we can find a suitable solution for your need.

That works with pytest-lazy-fixture

@pytest.mark.asyncio
@pytest.mark.parametrize('case', pytest.lazy_fixture((
    'case_1',
    'case_2'
)))
async def test_sum_of(case):
    a, b, c = case
    assert sum(a, b) == c

Thanks in advance.

@smarie
Copy link
Owner

smarie commented Jan 10, 2021

thanks for the feedback @HMaker ! I'll have a look

By the way note that you can directly unpack a, b and c as for pytest.mark.parametrize:

@parametrize_with_cases("a,b,c", cases='.')
async def test_sum_of(a, b, c):
    assert sum(a, b) == c

@smarie smarie closed this as completed in ce9b413 Jan 25, 2021
@smarie smarie reopened this Jan 25, 2021
@smarie
Copy link
Owner

smarie commented Jan 25, 2021

(Unfortunately I only fixed half of the issue yet: reopening)

@smarie smarie closed this as completed in 3837c6d Jan 25, 2021
smarie pushed a commit that referenced this issue Jan 25, 2021
@smarie
Copy link
Owner

smarie commented Jan 28, 2021

@HMaker can you confirm that this new version 3.2.0 works for you ? thanks !

@smarie
Copy link
Owner

smarie commented Feb 19, 2021

Up @HMaker ?

@HMaker
Copy link
Author

HMaker commented Feb 25, 2021

Up @HMaker ?

Sorry for late answer, I ran the code I sent above and the same error still happens. Steps to reproduce (linux env):

mkdir -p /temp/test-pytest
cd /tmp/test-pytest
python3 -m venv --prompt pytest venv
source venv/bin/activate
pip install pytest pytest-asyncio pytest-cases
cat << EOF > test_case.py
import random
import pytest
from pytest_cases import parametrize_with_cases


def sum(a, b):
    return a + b

@pytest.fixture
def random_num():
    return random.randint(0, 65543)

def case_1(random_num: int):
    return 1, random_num, 1 + random_num

def case_2(random_num: int):
    return 2, random_num, 2 + random_num

@pytest.mark.asyncio
@parametrize_with_cases('case', cases='.')
async def test_sum_of(case):
    a, b, c = case
    assert sum(a, b) == c
EOF
pytest test_case.py

and this is the error report from pytest

________________________________________________________________________ ERROR at setup of test_sum_of[2] ________________________________________________________________________

item = <Function test_sum_of[2]>

    def pytest_runtest_setup(item):
        if 'asyncio' in item.keywords:
            # inject an event loop fixture for all async tests
            if 'event_loop' in item.fixturenames:
                item.fixturenames.remove('event_loop')
>           item.fixturenames.insert(0, 'event_loop')

venv/lib/python3.9/site-packages/pytest_asyncio/plugin.py:195: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = SuperClosure with 2 alternative closures:
 - ['test_sum_of_case', '_1', 'random_num', 'request'] (filters: test_sum_of...e fixture tree is :
(test_sum_of_case) split: test_sum_of_case
 -  (_1,random_num,request)
 -  (_2,random_num,request)

index = 0, fixture_name = 'event_loop'

    def insert(self, index, fixture_name):
        # if index == 0:
        #     # prepend
        #     self.tree.fixture_defs
        if index == len(self):
            # append: supported
            self.tree.build_closure((fixture_name,))
            # update fixture defs
            self._update_fixture_defs()
        else:
>           raise NotImplementedError("When fixture unions are present, inserting a fixture in the closure is "
                                      "non-trivial. Please report this so that we can find a suitable solution for "
                                      "your need.")
E           NotImplementedError: When fixture unions are present, inserting a fixture in the closure is non-trivial. Please report this so that we can find a suitable solution for your need.

venv/lib/python3.9/site-packages/pytest_cases/plugin.py:631: NotImplementedError

That event_loop fixture is from pytest-asyncio. That error happened with Python 3.9.1 and following deps from pip freeze

attrs==20.3.0
decopatch==1.4.8
iniconfig==1.1.1
makefun==1.9.5
packaging==20.9
pluggy==0.13.1
py==1.10.0
pyparsing==2.4.7
pytest==6.2.2
pytest-asyncio==0.14.0
pytest-cases==3.2.1
toml==0.10.2

@smarie
Copy link
Owner

smarie commented Feb 26, 2021

Lol it seems that pytest-asyncio inserts it at the beginning of the closure instead of putting it at the end (append). Maybe they changed behaviour in the lastest version ? Or I just dreamed it, but I thought I tested it locally...

This will make it a little bit more complex to fix. I'll have to have a go again, thanks a LOT for reporting it !

@smarie smarie reopened this Feb 26, 2021
@HMaker
Copy link
Author

HMaker commented Feb 28, 2021

pytest-asyncio 0.14.0 was released in Jun 23, 2020 as shown in its pypi page, so I was using that version (actually it's set to "*" in requirements file, i.e. unpinned) when reported that error for the first time. Anyway, thanks for your attention.

@HMaker HMaker changed the title How to parametrize test functions marked by pytest-asyncio? Can't parametrize test functions marked by pytest-asyncio Feb 28, 2021
@smarie
Copy link
Owner

smarie commented Mar 2, 2021

Yes I saw that. Sorry, I don"t know what have happened. Anyway, I'll try to fix this now, and integrate the fix in the test suite if I can so that at least an independent machine checks it :) I was reluctant to add yet another plugin to the test suite but I guess I have not much choice to ensure proper coverage

@smarie
Copy link
Owner

smarie commented Mar 2, 2021

Confirmed: I was using pytest-asyncio 0.10.0 locally, and the fix works correctly for it. I'll now fix it for 0.14.0

@smarie smarie changed the title Can't parametrize test functions marked by pytest-asyncio Support pytest plugins that modify the fixture closure list Mar 2, 2021
@smarie
Copy link
Owner

smarie commented Mar 2, 2021

I made it work. Here are the current limitations, that should hopefully be quite tolerant to most existing plugins:

  • Removing a fixture from the closure using closure.remove(fixture_name) is supported as long as the fixture_name or one of its dependencies is not a fixture union (no tree)
  • Appending a fixture to the closure with closure.append(fixture_name) or closure.insert(len(closure), fixture_name) is supported in all cases (even if fixture_name or one of its dependencies is a fixture union and induces a tree with closure partitions))
  • Prepending a fixture to the closure with closure.insert(0, fixture_name) is supported as long as the fixture_name or one of its dependencies is not a fixture union (no tree)
  • Inserting a fixture anywhere else in the closure with closure.insert(i, fixture_name) is not supported, as it is non-trivial and context-dependent to understand "where and if" in all tree nodes this should have an impact.

I'll try to cut a release soon. Thanks again @HMaker for your feedback !

@smarie smarie closed this as completed in c29d9ff Mar 2, 2021
@smarie
Copy link
Owner

smarie commented Mar 23, 2021

@HMaker I managed to finally release version 3.3.0 that contains the fix. Sorry for the delay, I had to migrate from Travis to Github Actions to get my CI/CD pipelines back.

Could you let me know if this is working for you ? Just in case the tested configuration is slightly different from the one you're using.

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

No branches or pull requests

2 participants