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

mock side_effect should be checked for iterable not callable #80779

Closed
jazzblue mannequin opened this issue Apr 11, 2019 · 4 comments
Closed

mock side_effect should be checked for iterable not callable #80779

jazzblue mannequin opened this issue Apr 11, 2019 · 4 comments
Labels
3.8 only security fixes tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@jazzblue
Copy link
Mannequin

jazzblue mannequin commented Apr 11, 2019

BPO 36598
Nosy @tirkarthi, @jazzblue

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 = <Date 2019-04-12.10:09:13.377>
created_at = <Date 2019-04-11.04:12:58.459>
labels = ['3.8', 'type-bug', 'tests', 'invalid']
title = 'mock side_effect should be checked for iterable not callable'
updated_at = <Date 2019-04-12.10:09:13.374>
user = 'https://github.com/jazzblue'

bugs.python.org fields:

activity = <Date 2019-04-12.10:09:13.374>
actor = 'xtreak'
assignee = 'none'
closed = True
closed_date = <Date 2019-04-12.10:09:13.377>
closer = 'xtreak'
components = ['Tests']
creation = <Date 2019-04-11.04:12:58.459>
creator = 'jazzblue'
dependencies = []
files = []
hgrepos = []
issue_num = 36598
keywords = ['patch']
message_count = 4.0
messages = ['339923', '339927', '339992', '340018']
nosy_count = 2.0
nosy_names = ['xtreak', 'jazzblue']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue36598'
versions = ['Python 3.8']

@jazzblue
Copy link
Mannequin Author

jazzblue mannequin commented Apr 11, 2019

In mock.py, in method:
def _mock_call(_mock_self, *args, **kwargs):

There is a following piece of code:

    if not _callable(effect):
        result = next(effect)
        if _is_exception(result):
            raise result
        if result is DEFAULT:
            result = self.return_value
        return result

    ret_val = effect(*args, **kwargs)

This works correctly for iterables (such as lists) that are not defined as generators.
However, if one defined a generator as a function this would not work.

It seems like the check should be not for callable, but for iterable:

try:
    iter(effect)
except TypeError:
    # If not iterable then callable or exception
    if _callable(effect):
        ret_val = effect(*args, **kwargs)
    else:
        raise effect

else:  # Iterable
    result = next(effect)
    if _is_exception(result):
        raise result
    if result is DEFAULT:
        result = self.return_value
    return result

@jazzblue jazzblue mannequin added tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error labels Apr 11, 2019
@tirkarthi tirkarthi added the 3.8 only security fixes label Apr 11, 2019
@tirkarthi
Copy link
Member

I am not sure if the snippets you are referring to are from testing-cabal/mock repo which could be different from master branch. Current code is at [0]

if effect is not None:
    if _is_exception(effect):
        raise effect
    elif not _callable(effect):
        result = next(effect)
        if _is_exception(result):
            raise result
    else:
        result = effect(*args, **kwargs)

    if result is not DEFAULT:
        return result

This works correctly for iterables (such as lists) that are not defined as generators.
However, if one defined a generator as a function this would not work.

This does seem to work for generator function as below. Sorry, maybe I am getting it wrong with respect to terminologies and understanding the issue. Can you add a short script around what you are expecting?

$ cat ../backups/bpo36598.py
from unittest.mock import patch
def gen(i):
    while i < 5:
        yield i
        i += 1

def foo():
    return 1

with patch('__main__.foo', side_effect=gen(0)):
    for _ in range(2):
        print(foo())
    for _ in range(2):
        print(foo())
$ ./python.exe ../backups/bpo36598.py
0
1
2
3

[0]

if effect is not None:

@jazzblue
Copy link
Mannequin Author

jazzblue mannequin commented Apr 11, 2019

You are right. I was not calling generator the right way in mock. After I tried your suggestion it works.

@tirkarthi
Copy link
Member

Thanks, I am closing this as not a bug. Feel free to reopen this if I have missed any.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.8 only security fixes tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

1 participant