-
Notifications
You must be signed in to change notification settings - Fork 3.1k
tests: fix, cleanup and restructure test code #2569
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
Conversation
- remove unused imports in test cases - fix code smells and code style in conftest.py
342f1e0
to
5ddb3cf
Compare
cd5fd28
to
4c0d5a8
Compare
4c0d5a8
to
fb0fc33
Compare
68b5968
to
1e9745f
Compare
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.
Nice work, lots of files so respect going trough!
Hope that remove_all_containers
did not struck your machine. Good catch.
Just some minor styling comments, maybe you did that on purpose, then just discard my suggestions.
There is a gitignore file which seems to have no purpose.
WEB_PORTS
and VIRTUAL_HOST
sometimes have quotations and other not. Maybe also a good moment to lint that?
test/test_vhost-in-multiple-networks/test_vhost-in-multiple-networks.py
Outdated
Show resolved
Hide resolved
@SchoNie thanks for the feedback 🙏 Regarding the blank line between standard library and third party imports, that's what the PyCharm linter does by default, I think it's applying the PEP8 style guide:
I've removed it.
I've cleaned
That's planned in the next PR 👍 |
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.
Just 1 missed WEB_PORTS test_debug-endpoint/test_global.yml Line 51
Regarding the blank line between standard library and third party imports, that's what the PyCharm linter does by default, I think it's applying the PEP8 style guide:
Imports should be grouped in the following order:
- Standard library imports.
- Related third party imports.
- Local application/library specific imports.
You should put a blank line between each group of imports.
Nice, didn't know that, learning everyday! 🧑🎓
This PR fixe various minor or code style issues in the test code base:
report.longrepr
is now always aReprExceptionInfo
, see TestReport.longrepr is a string when pytest.fail(..., pytrace=False) is used pytest-dev/pytest#1316 and its fixreport.longrepr.addsection()
second parameter should be astr
, butcontainer.logs()
returnsbytes
packaging
andurllib3
as explicit dependencieslog.warn
(deprecated) ->log.warning
conftest.py
where relevantThe test files structure has also been changed so that we no longer have test files at the root of
test
and others in their own directory (all test files are in their own subdirectory now), and the name of the files themselves has been somewhat standardized.@SchoNie if you happen to have the time (and since you're the only one regularly contributing test code), a review would be nice 👍