-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Extract formatting tests #1785
Extract formatting tests #1785
Conversation
pytest-cases >= 2.3.0 | ||
coverage >= 5.3 | ||
parameterized >= 0.7.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather then adding a new dependency, can @pytest.mark.parametrize
be used to do the same thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can, but it has been requested specifically to avoid using pytest
in tests directly in order to allow people to use the testing platform of their choosing.
I personally prefer using pytest
at any time, but I respect the design decisions of this project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My main pytest dislike is it's magical patching of thing so I guess the adding of a dep to allow both testing systems to be used works. Especially when this refactor is a big win imo.
@cooperlees, any feedback on this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks like a good refactor to me. ~250 less lines and all makes sense! Thanks!
* Update test versions * Use parametrize to remove tests duplications * Extract sources format tests * Fix mypy errors * Fix .travis.yml
A lot of the tests in Black are exact duplications of other tests.
In order to remove some of the duplications, added 2 parametrized tests that combine those tests.