-
Notifications
You must be signed in to change notification settings - Fork 198
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
Change Django Tests to pytests #133
Conversation
@sbs2001 Thanks for this PR! Since all tests are now being discovered and run with one command, can you also remove the old call to pytest from I think the warnings are unrelated to your changes. My guess is that pytest shows them by default whereas the Django test runner doesn't. Two dependencies (whitenoise and Django REST Framework) use features of Django that will be removed in a future version. It is possible that there are more recent versions of those apps that address these warnings. You can try changing their versions in In the chat you wrote:
I think you use them correctly. However, the marks are only used in And regarding the fixtures; Having them in a separate file increases the distance to the code using them and arguably makes it harder to read the tests. @pombredanne commented elsewhere that he prefers tests to be self-contained if possible. Personally I don't mind it and prefer to avoid duplication if that is the tradeoff to be made. Maybe @pombredanne wants to chime in on this? Otherwise, if we do leave it as is, would you mind renaming the file from I didn't expect you to change all the existing tests from |
@haikoschol I have corrected the code. I changed the names to avoid name clashes. The warnings are not fixed though. Regarding conftest.py , after researching albeit I don't think we can change that to fixtures.py . I used a separate file because we need the same fixtures in multiple test files.Thanks for the detailed review! |
We can look into that after merging.
I didn't realize that the filename has special meaning for pytest. In that case we'll leave it as is. |
Signed-off-by: sbs2001 <shivam.sandbhor@gmail.com>
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.
Looks good now. @sbs2001 thanks again for contributing!
Signed-off-by: sbs2001 shivam.sandbhor@gmail.com
I have used this pytest extension. We are able to run all the tests simply by running
pytest
in the root directory.I am currently working on fixing the following warnings(I could use some help here).