Replace self.mock_* pairs by self.mocked_* context managers#5157
Replace self.mock_* pairs by self.mocked_* context managers#5157Flamefire wants to merge 1 commit into
self.mock_* pairs by self.mocked_* context managers#5157Conversation
Flamefire
force-pushed
the
mocked-stdstream
branch
2 times, most recently
from
6bb272a to
359f379
Compare
Flamefire
force-pushed
the
mocked-stdstream
branch
from
359f379 to
70c1333
Compare
Make sure cleanup happens
Flamefire
force-pushed
the
mocked-stdstream
branch
from
70c1333 to
d78dd8c
Compare
|
|
||
| @contextmanager | ||
| def mocked_stdout(self): | ||
| def mocked_stdout(self, force_tty=False): |
There was a problem hiding this comment.
force_tty should be used when turning the mocked pipe ON and not when turning it OFF (enabling it on the normal stdout/stderr should be a separate operation if ever needed)
There was a problem hiding this comment.
This is the function that turns the mocked pipe ON. Or do you mean something different?
There was a problem hiding this comment.
I mean it should be
self.mock_stdout(True, force_tty=force_tty)
...
self.mock_stdout(False)
instead of the other way around
wanted to suggest it as a change but i think the tool does not like suggestions going over deleted lines(?) and was giving weird stuff
|
|
||
| @contextmanager | ||
| def mocked_stderr(self): | ||
| def mocked_stderr(self, force_tty=False): |
| self.mock_stderr(False) | ||
| with self.mocked_stdout_stderr(): | ||
| self.assertErrorRegex(EasyBuildError, error_pattern, eb.fetch_step) | ||
| stderr = self.get_stderr().strip() |
There was a problem hiding this comment.
I assume here it was originally an error where the stdout mocking was not being turned off?
There was a problem hiding this comment.
Exactly. That's one of the reasons I want to get rid of the manual turn-on/off as soon as possible.
| self.assertTrue(re.match('^[0-9a-f]{40}$', read_file(shafile))) | ||
| self.assertExists(os.path.join(repodir, 'easybuild', 'easyblocks', '__init__.py')) | ||
| self.mock_stdout(False) | ||
| # stdout capture released by context manager above |
3 participants
Make sure cleanup happens
Followup to #5125 by adding the
force_ttyparameter to the context managersShould be merged soon as this is prone to merge conflicts