-
-
Notifications
You must be signed in to change notification settings - Fork 361
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
fix tests for pytest6.x, cherrypy/test/test_config.py::ConfigTests::testHandlerToolConfigOverride and cherrypy/test/test_caching.py::CacheTest::testGzipStaticCache #1897
Conversation
cherrypy/test/test_config.py
Outdated
@@ -221,8 +221,9 @@ def testHandlerToolConfigOverride(self): | |||
# the favicon in the page handler to be '../favicon.ico', | |||
# but then overrode it in config to be './static/dirback.jpg'. | |||
self.getPage('/favicon.ico') | |||
self.assertBody(open(os.path.join(localDir, 'static/dirback.jpg'), | |||
'rb').read()) | |||
testfile = open(os.path.join(localDir, 'static/dirback.jpg'),'rb') |
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.
This should use a CM (with
) in order to guarantee that the file descriptor gets closed always.
cherrypy/lib/__init__.py
Outdated
@@ -70,6 +70,10 @@ def __next__(self): | |||
raise StopIteration() | |||
next = __next__ | |||
|
|||
def __del__(self): | |||
"""Close input on descturct""" | |||
self.input.close() |
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.
I guess this should follow the same "if hasattr" approach as above.
cherrypy/lib/__init__.py
Outdated
@@ -70,6 +70,10 @@ def __next__(self): | |||
raise StopIteration() | |||
next = __next__ | |||
|
|||
def __del__(self): | |||
"""Close input on descturct""" |
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.
PEP257-compliant docstrings should end with a period.
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.
with open(os.path.join(localDir, 'static/dirback.jpg'), 'rb') as tf: | ||
self.assertBody(tf.read()) |
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'd probably be cleaner with pathlib
API. But it doesn't matter now. Just something to keep in mind for the future.
I think I need to apply put ignores in the pytest config and then address the warnings one by one... Ref cherrypy/cheroot@3eebb80 |
I'll rebase this PR meanwhile. |
What kind of change does this PR introduce?
What is the related issue number (starting with
#
)What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Stops two test failures with pytest6.x
Other information:
Checklist:
and description in grammatically correct, complete sentences