-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
bpo-32234: Context manager available for mailbox instances #4770
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
Changes from 2 commits
8d1a9aa
47fd0fe
ae45e88
382e57f
fd4d240
3c68123
755dc1c
bb790e5
ac9f696
2131714
6814da0
bef0829
49403ae
3582862
bbddd8a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -532,6 +532,11 @@ def _test_flush_or_close(self, method, should_call_close): | |
| self.assertIn(self._box.get_string(key), contents) | ||
| oldbox.close() | ||
|
|
||
| def test_use_context_manager(self): | ||
| # Mailboxes are usable as a context manager | ||
| with self._factory(self._path) as box: | ||
| self.assertIsInstance(box, self._box.__class__) | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a test for the lock?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @matrixise I'm unsure how to do it right. For example:
It would be nice to have a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good question, I don't remember your code, but I am going to assign myself your PR.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, thank you!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be tested in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this test is redundant now with the new test.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The other test is only for the subclasses that have
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, that makes sense. |
||
| def test_dump_message(self): | ||
| # Write message representations to disk | ||
| for input in (email.message_from_string(_sample_message), | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.