-
Notifications
You must be signed in to change notification settings - Fork 8
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 resource warnings in tests. #19
base: master
Are you sure you want to change the base?
Conversation
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'm in favor of fixing resource warnings! The changes in fileresult.txt
look fine, but I'm concerned that the changes to FileView
(returning a string instead of an open file) are pretty fundamental. In WSGI, it's the caller's responsibility to close()
the returned body content. Maybe this is masking an issue elsewhere?
file = open(fn, 'rb') | ||
return file | ||
with open(fn, 'rb') as file: | ||
return file.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.
I'm not sure this is testing what it was testing before. Because the line above writes a constant to the file, this whole function could be simplified to just return b'Hello\nWorld!\n'
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.
You are right, it now does something completely different. I'll dig into the code to find out where the file should be closed as it seems to be a feature to return a file handle from a view.
@@ -37,8 +37,8 @@ def __call__(self): | |||
fn = tempfile.mktemp() |
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.
Bad test is not cleaning after itself! And creates completely anonymous temporary files.
No description provided.