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

'parametrize_with_cases' does not work with 'cases=AUTO2' param #140

Closed
romanponomaryov opened this issue Oct 12, 2020 · 9 comments
Closed

Comments

@romanponomaryov
Copy link

romanponomaryov commented Oct 12, 2020

Hi, Sylvain!

The problem is as follows:
I have a file 'test_<name>.py' with test functions. In the same directory I also have a file with cases, which is called 'cases_<name>.py'.
If I use in 'test_<name>.py':

@parametrize_with_cases('my_var', cases=AUTO2)
def test_my_func(my_var):
   ...

then when I run my tests I get an error:

ValueError: Error importing test cases module to parametrize function <function test_my_func at 0x...>: 
unable to import AUTO cases module '<path_to_module>.test_<name>_cases'. 

../../../venv/lib/python3.6/site-packages/pytest_cases/case_parametrizer_new.py:459: ValueError

Mind the "unable to import AUTO" - it's supposed to be "AUTO2" as I understand..

If I use 'test_<name>_cases.py' as a name for the file with cases and "cases=AUTO" or do not mention the "cases" param - all works fine.
If I delete or rename the file with cases and run the tests with "cases=AUTO2" - I get:

ValueError: Error importing test cases module to parametrize function <function test_my_func at 0x...>: 
unable to import AUTO2 cases module '<path_to_module>.cases_<name>'. 

So - this time AUTO2 is mentioned correctly and the name of the file pytest is trying to find is as expected.
Could you please look into this?

On another note, it seems weird that I have to import AUTO2 from pytest_cases.common_others to be able to use it in 'cases' parameter.
It would be much more intuitive to submit it as a string: @parametrize_with_cases('my_var', cases='AUTO2'). Or maybe I'm doing something wrong here?

I'm using pytest 6.0.1 and pytest-cases 2.2.5

Thanks in advance for your answer

@smarie
Copy link
Owner

smarie commented Oct 13, 2020

Thanks @romanponomaryov for reporting this !

Indeed it seems weird. I tried to reproduce the error but could not, unfortunately :(

Can you try to do this in debug mode, and to put a breakpoint at the beginning of your cases_<name>.py file ? I suspect that there is an error in your cases file, which leads to an import error, which is then wrapped into the valueerror and maybe not displayed in the stack trace ?

Also concerning the string variant, I am not against it. However consider that users already import parametrize_with_cases so adding another import is just as quick as <alt+enter> in pycharm. But of course we should not take IDE into account when writing libs, so I agree with you.

@romanponomaryov
Copy link
Author

romanponomaryov commented Oct 13, 2020

I don't think there's a problem with my cases file, as

If I use 'test_<name>_cases.py' as a name for the file with cases and "cases=AUTO" or do not mention the "cases" param - all works fine.

I created 'test_experiment.py' which has only one function from my initial file with tests ('test_<name>.py')
and
renamed my initial file with cases into 'cases_experiment.py'
and
tried to run such a configuration with "cases=AUTO2" in parametrize_with_cases and - it worked!
So, the problem seems to be somewhere in my 'test_<name>.py'.
I will try to narrow it down and report my findings.

@romanponomaryov
Copy link
Author

It turns out that in my 'test_<name>.py' I had another test function:

@parametrize_with_cases('my_var_2')
def test_my_func_2(my_var_2):
    ...

And this was causing the issue. When I updated 'cases=AUTO2' for this function as well - the issue went away.
So, if one is using 'cases=AUTO2' (and thus 'cases_<name>.py' naming) - they have to ensure that it is explicitly used in all test functions that use parametrize_with_cases in this file, otherwise running any test function with parametrize_with_cases in this file will result in the aforementioned ValueError.
This seems like expected behavior, but might still deserve a mention in the documentation, as the error is tricky to debug.
I think this issue can be closed.

@romanponomaryov
Copy link
Author

One more note concerning the string variant (cases=AUTO2 vs cases="AUTO2"). It's not a problem to make an import of AUTO2, but to figure out that I needed to make it in order to use it as a value for the parameter took some time and seemed counterintuitive. Maybe it's just me, though.

@smarie
Copy link
Owner

smarie commented Oct 14, 2020

Thanks for the detailed feedback @romanponomaryov ! Glad you could find a way to fix this

I'll leave this ticket open for later times (I will soon be inactive for family reasons for a couple of weeks), so that I can try to imagine a way to improve the user experience for the future.

Maybe one way would be to automate the search completely (merging AUTO2 into AUTO so that both names are searched for in a row ?). An alternative could be to explicitly declare the AUTO2 mode in the module somewhere (in a global variable or in a method called at the top of the module)... If you have other ideas do not hesitate to post them here !

@romanponomaryov
Copy link
Author

Ideally, three cases can be satisfied here:

  1. A convenient default way of cases discovery.
  2. The way to tweak cases discovery for all tests without explicitly declaring it in every test.
  3. The way to set different cases discovery for some tests.

How to do this though...
Maybe:

  • for cases 1 and 2 - introduce a pytest wide setting, for example in pytest.ini, using something like "cases_file_prefix" and "cases_file_suffix" parameters with defaults ["cases_"] and ["_cases"] respectively and the ability to change them.
  • for case 3 - introduce @cases_file_name decorator with same "cases_file_prefix" and "cases_file_suffix" parameters that would override the pytest wide settings. This decorator could be used on test functions and classes.

@smarie
Copy link
Owner

smarie commented Oct 23, 2020

Thanks for the proposal @romanponomaryov !

I very much like your pytest.ini suggestion. I like less the additional decorator, since users already have to use @parametrize_with_cases. That being said the new decorator could be helpful to change things on a whole class as you suggest. I'll think about it when I have again some dev time to spend on this (in a few weeks)

@romanponomaryov
Copy link
Author

New decorator is not needed per se for case 3. Same thing can be achieved e.g. via having same parameters "cases_file_prefix" and "cases_file_suffix" in @parametrize_with_cases. And maybe @parametrize_with_cases can be made to work with test classes.

@smarie smarie closed this as completed in aeb72ef Dec 16, 2020
@smarie
Copy link
Owner

smarie commented Dec 16, 2020

Hi @romanponomaryov , for now I have implemented a basic fusion of both naming convention: now AUTO (default) will look for test_<name>_cases.py, then cases_<name>.py.

This will be released in 3.1.0.

Feel free to open a new ticket referencing this if there is any other of your suggestion you would like to pursue.

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