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

Unnecessary prefix in test ids when fixture_ref is used in parametrize_plus #69

Closed
smarie opened this issue Dec 18, 2019 · 8 comments
Closed

Comments

@smarie
Copy link
Owner

smarie commented Dec 18, 2019

@smarie Thank you for pytest-cases, i've managed to get what i wanted, and it looks almost fine, except for fixture names :)

import pytest
from pytest_lambda import lambda_fixture
from pytest_cases import fixture_ref, pytest_parametrize_plus

from . import models


def lazy_fixture_generator(mod):
    """
    Lazy = lazy_fixture_generator(globals())
    """
    ids = (x for x in range(1000))

    def Lazy(fun):
        """
        Lazy fixture loading from lambda

        Usage:
        @pytest_parametrize_plus("args", [Lazy(lambda f1, f2: (f1.atrr, f2.attr))])
        """
        name = f"lazy{next(ids)}"
        mod[name] = lambda_fixture(fun)
        return fixture_ref(name)
    return Lazy


Lazy = lazy_fixture_generator(globals())


@pytest.fixture
def book1():
    return models.Book(name="Moby Dick")


@pytest.fixture
def book2():
    return models.Book(name="Alice in Wonderland")


@pytest_parametrize_plus("name, expects", [
    (Lazy(lambda book1: book1.name), "Moby Dick"),
    (Lazy(lambda book2: book2.name), "Alice in Wonderland"),
])
@pytest.mark.django_db
def test_get_or_create_book(name, expects):
    book = models.Book.objects.get_or_create(name=name)[0]
    assert book.name == expects
test output
 pipenv run pytest -vv --disable-warnings
Test session starts (platform: linux, Python 3.8.0, pytest 4.5.0, pytest-sugar 0.9.2)
cachedir: .pytest_cache
Django settings: bookstore.settings (from ini file)
rootdir: /home/serg/work/git/example/src/bookstore, inifile: pytest.ini
plugins: django-3.7.0, sugar-0.9.2, lambda-1.1.0, cases-1.11.9
collecting ... 
 core/tests.py::test_get_or_create_book[test_get_or_create_book_name_expects_is_fixtureproduct__0] ✓50% █████     
 core/tests.py::test_get_or_create_book[test_get_or_create_book_name_expects_is_fixtureproduct__1] ✓100% ██████████

Results (0.45s):
       2 passed

Originally posted by @last-partizan in pytest-dev/pytest#349 (comment)

@last-partizan
Copy link

My first attempt to fix it was to wrap fixture_ref in pytest.param with explicit id, but pytest-cases expects values to be tuple, not ParameterSet.

In current output there is clearly unneded prefix test_get_or_create, do you think it can be removed? or it serves some purpose elsewhere?

@smarie
Copy link
Owner Author

smarie commented Dec 18, 2019

So you issue is that the generated test id name is a bit unfriendly ;) (test_get_or_create_book_name_expects_is_fixtureproduct__0).

Well I agree, but at the same time I have no clue on how to do better here. Here is how I proceed:

  • when @pytest_parametrize_plus is used for a single parameter and contains at least one fixture_ref() in its values, the function is not parametrized as usual. Instead, a new "union" fixture is created, and injected in the test function arguments.

  • the new "union" fixture that is created is the union of several fixtures: some containing the parameters, and the ones explicitly mentioned as fixture_ref(). Therefore

@pytest_parametrize_plus("name", [
    fixture_ref(book1_name),
    'hi',
    'ih',
    fixture_ref(book2_name),
])
def test_get_or_create_book(name):
    ...

will create a union of 3 fixtures: book1 (with ids 'A' and 'B'), test_get_or_create_book_name_is_1to2 (with ids 'hi' and 'ih'), and book2

This union is named uniquely after the test function name and the parameter name. So it is named: test_get_or_create_book_name (<test_func_name><param_name>). And it is parametrized with 3 values (book1, test_get_or_create_book_name_is_1to2, book2). To make the final ids understandable, I customize them with a <union_fixture>_is_<blah> style, so the resulting list looks like this:

test_get_or_create_book[test_get_or_create_book_name_is_book1-A]
test_get_or_create_book[test_get_or_create_book_name_is_book1-B]
test_get_or_create_book[test_get_or_create_book_name_is_1to2-hi]
test_get_or_create_book[test_get_or_create_book_name_is_1to2-ih]
test_get_or_create_book[test_get_or_create_book_name_is_book2]
  • now when the parameters provided are tuples (as in your example), it is even mode complex. I was a bit short on ideas here, so I used 'fixtureproduct' for all those cases, because each fixture in the union is now potentially a product of fixtures.

Any ideas are more than welcome ! It is hard to make things simple here :)

@smarie
Copy link
Owner Author

smarie commented Dec 18, 2019

Concerning your suggestions:

  • ParameterSet should be supported in the list of values but I do not know if I correctly copy the id. If not could you please file an issue here ?

  • great idea to remove the unneeded prefix actually: I did not realize that since I already use custom ids (and not the fixture names, that need the prefix to be unique), I could actually do it. Will do tomorrow ! Thanks

@last-partizan
Copy link

last-partizan commented Dec 18, 2019

@pytest_parametrize_plus("name, expects", [
    (pytest.param(Lazy(lambda book2: book2.name), name='alice'), "Alice in Wonderland"),
    pytest.param(Lazy(lambda book1: book1.name), "Moby Dick", id="mobydick"),
    pytest.param("explicit", "explicit", id='ee'),
])
@pytest.mark.django_db
def test_get_or_create_book(name, expects):
    book = models.Book.objects.get_or_create(name=name)[0]
    assert book.name == expects

First case is working, but yes, name is copied incorrectly, i get 'name0' instead of 'alice', i'll open separate issue.
Second and third cases throwing errors:

ValueError: Invalid parameter values containing 3 items while the number of parameters is 2: ParameterSet(values=('explicit', 'explicit'), marks=(), id='ee').

Do they supposed to throw it or can it be fixed? By my understanding, it can, but with my first idea ids will be lost:

for i, v in enumerate(argvalues):
try:
j = 0
fix_pos = []

            try:
                v = v.values
            except AttributeError:
                pass

@last-partizan
Copy link

Oh, i missed first message, and now i just read it.

Wait a minute, i was trying to do this:

    (pytest.param(fixture_ref('book1'), name='alice'), "Alice in Wonderland"),

And it passed collect stage, but crashed at test stage (same as simple pytest.mark.parametrize, so i assume this is fine, and i should not use tuples with pytest.param inside).

@last-partizan
Copy link

ParameterSet should be supported in the list of values but I do not know if I correctly copy the id. If not could you please file an issue here ?

By 'ParameterSet should be supported in the list of values' you mean this?

@pytest_parametrize_plus("name, expected", [
    (pytest.param(fixture_ref('book1'), id="something"), "something"),
]

Here is actually two issues: incorrect id, and it doesn't work (it passes ParameterSet to test_function), just like pytest, so here is nothing to fix.

When using pytest.param(fixture_ref("book1"), id="book1") ids is working fine.

@smarie
Copy link
Owner Author

smarie commented Dec 19, 2019

By 'ParameterSet should be supported in the list of values' you mean this? (...) pytest.param

Yes I meant pytest.param. So I understand that here pytest-cases works similar to pytest so there is no particular issue. Am I correct ? If so, please open the ticket in a separate issue with a minimal reproducing example + actual/expected outcome so that I can handle it properly.

Now concerning removing the prefix, I'm on it.

@last-partizan
Copy link

Yes, you are correct.

Thanks!

@smarie smarie changed the title pytest lambda and fixture names Unnecessary prefix in test ids when fixture_ref is used in parametrize_plus Dec 19, 2019
@smarie smarie closed this as completed in c63fe90 Dec 19, 2019
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