Hide __enter__ calls in mock_open

[CendioOssman]

BPO	44503
Nosy	@voidspace, @CendioOssman

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-06-24.11:08:55.790>
labels = ['type-feature', 'library']
title = 'Hide __enter__ calls in mock_open'
updated_at = <Date 2021-06-26.15:05:24.083>
user = 'https://github.com/CendioOssman'

bugs.python.org fields:

activity = <Date 2021-06-26.15:05:24.083>
actor = 'terry.reedy'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2021-06-24.11:08:55.790>
creator = 'CendioOssman'
dependencies = []
files = []
hgrepos = []
issue_num = 44503
keywords = []
message_count = 2.0
messages = ['396469', '396470']
nosy_count = 2.0
nosy_names = ['michael.foord', 'CendioOssman']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue44503'
versions = []

[CendioOssman]

I'd like to write this test case:

  with patch('builtins.open') as pyopen:
    mock_open(pyopen, read_data="foo")
    run()
    pyopen.assert_has_calls([call("filename", "wt"),
                             call().write("gazonk"),
                             call().close()])

and I shouldn't have to care if the code is written like this:

  def run():
    f = open("filename", "wt")
    try:
      write("gazonk")
    finally:
      f.close()

or like this:

  def run():
    with open("filename", "wt") as f:
      write("gazonk")

[CendioOssman]

Also see bpo-44185 for __exit__.

[tirkarthi]

The __enter__ and __exit__ calls are tested to be part of executed methods for the object and are recorded. Removing them might result in case where context manager style is changed to use manual close and the test behavior might not catch these. I don't think it's worth the cost to special case mock_open to behave differently filtering out dunder methods and along with explaining them in the docs since there could be existing code depending on this.

cpython/Lib/unittest/test/testmock/testwith.py

Lines 153 to 163 in 2fadde7

	def test_mock_open_context_manager(self):
	mock = mock_open()
	handle = mock.return_value
	with patch('%s.open' % __name__, mock, create=True):
	with open('foo') as f:
	f.read()
	expected_calls = [call('foo'), call().__enter__(), call().read(),
	call().__exit__(None, None, None)]
	self.assertEqual(mock.mock_calls, expected_calls)
	self.assertIs(f, handle)

[CendioOssman]

I can be sympathetic to existing code relying on current behaviour, so this could be gated by an option to mock_open(). But I think the existing behaviour is forcing us to write bad tests. We should be testing behaviour, not implementation.

I assume the whole point of mock_open() is to avoid forcing everyone to keep duplicating wrappers for handling the common case of opening a file. Which we are now forced to do since mock_open() isn't agnostic to how the file is closed.