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 Parameterized Pytest Tests #58

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tmr232
Copy link

@tmr232 tmr232 commented Dec 17, 2019

Description

This PR adds support for parameterized tests in Pytest.
Closes #55

The solution

This is achieved by using the request fixture to query the current test name, and using that with a new Namer to get the generated filenames to match the names printed by pytest.

Relevant tests have been added.

This commit adds support for parameterized tests in Pytest.
Closes approvals#55


@pytest.fixture
def pytest_verify(request):
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was unsure where to place this fixture, so I placed it here for now. It should probably move before the code is merged.

@emilybache
Copy link
Contributor

I had a good look at this pull request and thought about it and decided not to accept it as-is. I have pushed some changes today that add a PyTestNamer that uses the request fixture to work out where to put the approved files. I couldn't have written this without seeing your code first, so thankyou for the inspiration! If you use this namer, you get the effect you're after - that parameterized tests get the right name.

I havn't added the 'pytest_verify' fixture you have added. I didn't find a good way to include it. I don't want approvaltests to stop working for people who are using unittest only. I'm thinking I might put it in the PyTest plugin (https://github.com/approvals/ApprovalTests.Python.PytestPlugin)

At present it's a bit verbose to use the pytest namer. You need something like this:

def test_with_pytest_namer(request):
namer = PyTestNamer(request)
verify("foo", namer=namer)

I'm hoping I can improve on that, but this is at least possible.

@tmr232
Copy link
Author

tmr232 commented Dec 19, 2019

How about adding an approvaltests.pytest.fixtures module and put in there?
Then we'd be able to import from that module directly, without affecting the dependencies for the rest of the package.

As for the tests - I think we should include a parameterized test, as that was the issue that prompted the addition of a Pytest namer.

@emilybache
Copy link
Contributor

You make good points! I'm away over Christmas, and I will take this up again in the new year.

@tmr232
Copy link
Author

tmr232 commented Jan 19, 2020

@emilybache any updates?

@emilybache
Copy link
Contributor

Hi, sorry for the long silence. I havn't felt able to prioritize working on this recently. You've made some good suggestions and I need to spend some time looking into it.

@isidore
Copy link
Member

isidore commented Nov 21, 2021

@tmr232 We were looking at this today (sorry for the very long delay) if you would like can you join our mobbing session (sundays 10am MST) next week and we should be able to get this into the main branch and deploy it.

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

Successfully merging this pull request may close these issues.

Support parametrized pytest tests
3 participants