Use `pytest`'s fixtures to reduce repetition across tests
Pytest offers various features, one of them being the use of fixtures. This helps us better manage the tests and setup preconditions. Using the pytest.mark.parameterize fixture helps us give multiple inputs to test functions.
Learn more : https://docs.pytest.org/en/stable/how-to/parametrize.html.
We can take for example
assert isinstance(processed[1][0][0], np.ndarray)
assert isinstance(processed[1][0][1], np.ndarray)
assert isinstance(processed[1][1], np.ndarray)
inside a fixture and work up things accordingly.
@agriyakhetarpal We can put this for hacktoberfest.
I added the labels – additionally, could you please link to some code examples (a few low-hanging fruits) where you've noticed that this is the case, so that it's easier for potential contributors to know what to start with?
Hey would love to pick up this issue ! I understand that we must set up fixtures to handle multiple cases here and thereby consolidating tests for different inputs into a single, reusable function, therefore making the code more concise. Can I start working on this ?
Sure @Aswinr24, feel free to work on this and also don't hesitate to let us know if you need help at any point :)
Could You Please Review my PR and suggest if any changes are required
Hi @agriyakhetarpal, hope you're doing well! I've opened this PR as part of a series where I aim to parameterize tests wherever possible, following your suggestions. Could you please review this PR and let me know if any changes are needed?
Thank you!
Hi! I noticed some PRs already linked to this issue, is there still scope for additional refactoring using pytest fixtures in other test modules? I’d be happy to help with that.
Hi, @swastim01! Indeed, there is still scope for additional refactoring. There is #4401 that @prady0t previously started, but it wasn't touched for a while and was later closed. The idea is that we can move most of our shared.py test harnesses into conftest.py. I am open to hearing ideas on how this could be done. I suppose you could draw some inspiration from that PR in a PR of your own (while including @prady0t's previous commits, of course!)
@agriyakhetarpal, thanks for the guidance! I had a look at #4401, the full removal of shared.py was a bit too broad, but the idea of moving the reusable test setup into conftest.py seems workable.
I can take this forward in smaller steps by extracting the commonly used parts of shared.py into fixtures and updating tests gradually. Before I begin, could you confirm the preferred scope?
Should I start with tests/unit/parameters/ or widen it to other modules where repetition is obvious?
Thanks for thinking through this. I think the preferred scope is broad but the improvements are limited; we've done a lot of improvements for the test suite over the years. Starting with the parameters tests sounds best. Smaller but multiple PRs with obvious repetition removed will be easier for us to review – limiting to a few hundred lines of code and a few files at a time. We can keep the shared.py file to whatever extent its contents can't be removed.