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

AUTO cases in conftest: AssertionError #309

Closed
michele-riva opened this issue Sep 12, 2023 · 3 comments · Fixed by #320
Closed

AUTO cases in conftest: AssertionError #309

michele-riva opened this issue Sep 12, 2023 · 3 comments · Fixed by #320

Comments

@michele-riva
Copy link
Contributor

michele-riva commented Sep 12, 2023

Thanks for the great work! I just stumbled across a corner case.

Example to reproduce:

conftest.py

from pytest_cases import fixture, parametrize_with_cases

@fixture
@parametrize_with_cases('arg')
def auto_cases(arg):
    return arg

test_cases.py

def case():
    return 1

test.py

def test(auto_cases):
    assert auto_cases == 1

Gives

ImportError while loading conftest
pytest_cases/common_pytest.py:925: in apply
    return apply_decorator(test_or_fixture_func, container)
pytest_cases/case_parametrizer_new.py:143: in _apply_parametrization
    cases_funs = get_all_cases(f, cases=cases, prefix=prefix, glob=glob, has_tag=has_tag, filter=filter)
pytest_cases/case_parametrizer_new.py:303: in get_all_cases
    c = import_default_cases_module(caller_module_name)
pytest_cases/case_parametrizer_new.py:670: in import_default_cases_module
    assert parts[-1][0:5] == 'test_'
E   AssertionError: assert 'conft' == 'test_'

Seems very reasonable to me to not allow AUTO use in a conftest.py as it can be ambiguous which cases are to be used for parametrization. However, it probably would be nicer to explicitly check for this and raise a clearer exception? E.g., at the beginning of

def import_default_cases_module(test_module_name):

As a side note, I realized a possible inconsistency in the doc of the @parametrize_with_cases API: it looks like the "second alternative" for looking up cases in AUTO mode is cases_<name>.py (plural), and not case_<name>.py. See

cases_module_name2 = "%s.cases_%s" % ('.'.join(parts[:-1]), parts[-1][5:])

@smarie
Copy link
Owner

smarie commented Nov 19, 2023

Thanks @michele-riva ! Indeed I confirm

  • The docstring and API reference page state By default (cases=AUTO) the list of test cases is automatically drawn from the python module file named test_<name>_cases.py or if not found, case_<name>.py, where test_<name> is the current module name. while the correct alternate pattern is cases_<name>.py. Note that a few comments here and there in the code have to be fixed too as they mention 'case_'.
  • It would be beneficial to raise an explicit ValueError in get_all_cases just before import_default_cases_module is called (here) if test_module_name.split('.')[-1] does not start with test_, stating that AUTO is not allowed outside of normal test files.

Would you like to propose a PR for these two mods ?

@michele-riva
Copy link
Contributor Author

Sure, I can draft one right away.

@michele-riva
Copy link
Contributor Author

Fixed by PR #320

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants