[Refactor] Screenshot generator: Better state management via mocks

[kdmukai]

Motivation

The screenshot generator's ScreenshotConfig.run_before param currently provides a way to inject data into the Controller state just before a given screenshot is rendered.

The downside is that these state changes remain in place for all subsequent screenshots that follow in the screenshot_list.

Some screenshots depend on a prior state change and try to document this with # Relies on callback above comments.

A few other screenshots also depend on prior state changes but do NOT document the dependency.

This run_before method and its aftereffects is not ideal.

And when more complex contexts are required for new screenshots (see #858) the weaknesses here look even worse.

Solution

This PR refactors run_before to use temporary unittest.mock.patch calls instead. The patches are defined using the slightly less straightforward contextmanager python approach (I had not been terribly familiar with yield) and passed into ScreenshotConfig via a new mock_context_manager param. But the tradeoff win is that this approach brings familiar test suite mocking patterns into the screenshot generator.

Simplified example:

@contextmanager
def mock_override_foo_in_controller():
    bar = "some demo value"
    with patch.object(controller, 'foo', bar):
        yield

with mock_override_foo_in_controller():
    # Inside this context, controller.foo == bar
    # Run the View and the generate screenshot within this context.

The screenshots that were relying on prior state changes being present now just call whichever mock_context_manager they need. Those dependencies on a particular expected internal state are now explicit in all cases.

Interestingly, there were also some DRY wins.

---

This pull request is categorized as a:

• Code refactor

Checklist

• I’ve run pytest and made sure all unit tests pass before submitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

• N/A

I have tested this PR on the following platforms/os:

• N/A

[alvroble]

Maybe this is not the case, but if in the future a context manager needs to declare more variables, this could be a good alternative to avoid the withnesting:

from contextlib import ExitStack
with ExitStack() as stack:
    stack.enter_context(patch.object(controller, 'psbt', decoder.get_psbt()))
    stack.enter_context(patch.object(controller, 'psbt_seed', seed))
    stack.enter_context(patch.object(controller, 'psbt_parser', PSBTParser(p=controller.psbt, seed=seed)))
    yield

[kdmukai]

Interesting. Diving further into python techniques I've never used!

For the moment I like the with patch... convention (even with the nesting), because it directly parallels the same technique used elsewhere in the test suite.

[alvroble]

I understood the changes and I think it's a good QOL improvement. ACK tested.

[kdmukai]

Rebased to current dev.

[newtonick]

ACK and tested