Skip to content
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

then_enter_with(None) makes the stub return None instead of an enterable context manager #135

Closed
SyntaxColoring opened this issue Jun 3, 2022 · 2 comments · Fixed by #137
Labels
bug Something isn't working

Comments

@SyntaxColoring
Copy link
Contributor

SyntaxColoring commented Jun 3, 2022

If you have production code that looks like this:

with foo.bar(bar_arg) as baz:
    ...

Then its Decoy test might do:

bar_arg_captor = matchers.Captor()
decoy.when(foo.bar(bar_arg_captor())).then_enter_with("baz value")

But sometimes the with statement has no as clause:

with foo.bar(bar_arg):
    ...

I expected this to work:

bar_arg_captor = matchers.Captor()
decoy.when(foo.bar(bar_arg_captor())).then_enter_with(None)

But this causes the production code's call to foo.bar(bar_arg) to return None. It should instead return a context manager that returns None when it's entered.

It looks to me like this is an unintentional side-effect of the way StubBehavior works internally:

class StubBehavior(NamedTuple):
"""A recorded stub behavior."""
return_value: Optional[Any] = None
context_value: Optional[Any] = None
error: Optional[Exception] = None
action: Optional[Callable[..., Any]] = None
once: bool = False

It can't distinguish between being configured with a context_value that's None versus not being configured with a context_value at all.

To solve a similar problem, the implementation of dataclasses uses a special MISSING sentinel value that's distinct from None. Maybe a similar pattern could work here.

@SyntaxColoring SyntaxColoring changed the title then_enter_with(None) makes the not configure a context manager stub then_enter_with(None) makes the stub return None instead of an enterable context manager Jun 3, 2022
@SyntaxColoring
Copy link
Contributor Author

SyntaxColoring commented Jun 3, 2022

An easy workaround is to provide some arbitrary dummy value to then_enter_with(), like then_enter_with("<value unused>").

@mcous
Copy link
Owner

mcous commented Jun 4, 2022

Shoot, we need a sentinel for this to work.

Stubbing a None context value is interesting, though, because it seems to me like it would be difficult to construct a test that ensures a stubbed context manager is actually used. Since there's no value to interact with, a stubbed None can't validate proper usage. Though it would definitely have usage when combined with tests that do test the functionality to ensure unrelated tests can pass.

I think the better workaround here is to return a CM mock with its __enter__ etc. stubbed out, as in: https://mike.cousins.io/decoy/advanced/context-managers/#general-context-managers

# don't do this
decoy.when(mock_dep.get_context()).then_enter_with("<dummy_value>")

# do this
mock_cm = decoy.mock()
decoy.when(mock_dep.get_context()).then_return(mock_cm)
# ...
# pretty sure this will work, will test it out though
decoy.verify(
    mock_cm.__enter__(),
    # ...
)

@mcous mcous added the bug Something isn't working label Jun 4, 2022
@mcous mcous closed this as completed in #137 Jun 5, 2022
mcous added a commit that referenced this issue Jun 5, 2022
SyntaxColoring added a commit to Opentrons/opentrons that referenced this issue Jun 6, 2022
SyntaxColoring added a commit to Opentrons/opentrons that referenced this issue Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants