-
-
Notifications
You must be signed in to change notification settings - Fork 34.6k
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 12 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 |
|---|---|---|
|
|
@@ -542,6 +542,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._box as box: | ||
| self.assertIs(self._box, box) | ||
|
|
||
|
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), | ||
|
|
@@ -1122,6 +1127,16 @@ def test_ownership_after_flush(self): | |
| self.assertEqual(st.st_gid, other_gid) | ||
| self.assertEqual(st.st_mode, mode) | ||
|
|
||
| def test_context_manager_locks_and_closes(self): | ||
| # Context manager locks/unlocks and closes. | ||
| # (This test uses an implementation detail to get the state.) | ||
| self.assertFalse(self._box._locked) | ||
| with self._box as context_object: | ||
| self.assertIs(self._box, context_object) | ||
| self.assertTrue(self._box._locked) | ||
| self.assertFalse(self._box._file.closed) | ||
| self.assertFalse(self._box._locked) | ||
| self.assertTrue(self._box._file.closed) | ||
|
|
||
| class _TestMboxMMDF(_TestSingleFile): | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Mailbox instances can be used as a context manager. The Mailbox is locked | ||
| at enter and unlocked and closed at exit. | ||
|
encukou marked this conversation as resolved.
Outdated
|
||
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.