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

Add restricted mocks to the python unittest mocking framework #74726

Closed
mariocj89 mannequin opened this issue Jun 1, 2017 · 11 comments
Closed

Add restricted mocks to the python unittest mocking framework #74726

mariocj89 mannequin opened this issue Jun 1, 2017 · 11 comments
Labels
3.7 stdlib type-feature

Comments

@mariocj89
Copy link
Mannequin

@mariocj89 mariocj89 mannequin commented Jun 1, 2017

BPO 30541
Nosy @vstinner, @rbtcollins, @ericvw, @voidspace, @berkerpeksag, @grzgrzgrz3, @mariocj89
PRs
  • #1923
  • #5107
  • 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 2017-10-17.11:36:43.712>
    created_at = <Date 2017-06-01.20:16:56.139>
    labels = ['3.7', 'type-feature', 'library']
    title = 'Add restricted mocks to the python unittest mocking framework'
    updated_at = <Date 2018-01-05.17:42:54.900>
    user = 'https://github.com/mariocj89'

    bugs.python.org fields:

    activity = <Date 2018-01-05.17:42:54.900>
    actor = 'p-ganssle'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-10-17.11:36:43.712>
    closer = 'vstinner'
    components = ['Library (Lib)']
    creation = <Date 2017-06-01.20:16:56.139>
    creator = 'mariocj89'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 30541
    keywords = []
    message_count = 11.0
    messages = ['294961', '294969', '296159', '296573', '296589', '296600', '296609', '296697', '296705', '301230', '304498']
    nosy_count = 8.0
    nosy_names = ['vstinner', 'rbcollins', 'ericvw', 'michael.foord', 'berker.peksag', 'grzgrzgrz3', 'mariocj89', 'lkollar']
    pr_nums = ['1923', '5107']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue30541'
    versions = ['Python 3.7']

    @mariocj89
    Copy link
    Mannequin Author

    @mariocj89 mariocj89 mannequin commented Jun 1, 2017

    Define a way to disable the automatic generation of submocks when accessing an attribute of a mock which is not set.

    Rationale:
    Inspired by GMock RestrictedMock, it aims to allow the developer to declare a narrow interface to the mock that defines what the mocks allows to be called on.
    The feature of mocks returning mocks by default is extremely useful but not always desired. Quite often you rely on it only at the time you are writing the test but you want it to be disabled at the time the mock is passed into your code.

    It also prevents user errors when mocking incorrect paths or having typos when calling attributes/methods of the mock.
    We have tried it internally in our company and it gives quite a nicer user experience for many use cases, specially for new users of mock as it helps out when you mock the wrong path.

    Posible interfaces:
    New Mock type, SeledMock which can be used instead of the "common" mock that has an attribute "sealed" which once set to true disables the dynamic generation of "submocks"

    The final goal is to be able to write tests like:

    >> m = mock.Mock() # or = mock.SealableMock()
    >> m.method1.return_value.attr1.method2.return_value = 1
    >> mock.seal(m) # or mock.sealed = True

    >>> m.method1().attr1.method2()  # This path has been declared above
    # 1
    >>> m.method1().attr2  # This was not defined so it is going to raise a meaningful exception
    # Exception: SealedMockAttributeAccess: mock.method1().attr2

    @mariocj89 mariocj89 mannequin added 3.7 stdlib type-feature labels Jun 1, 2017
    @mariocj89
    Copy link
    Mannequin Author

    @mariocj89 mariocj89 mannequin commented Jun 1, 2017

    Sample implementation using the new class:
    mariocj89@2f13963

    Sample implementation using the new function to seal existing mocks:
    mariocj89@9ba039e

    Happy to submit a PR if the idea is accepted.

    The only benefit I see from the using a separate class is that you will be able to do:

    >> m = mock.SealedMock()
    >> m.important_attr = 42
    >> m.freeflow_attribute = mock.Mock()
    >> mock.seal(m)

    which will allow to define "subparts of the mock" without the sealing.

    That said I still prefer the function implementation as it looks much nicer (credit to Victor Stinner for the idea)

    @berkerpeksag
    Copy link
    Member

    @berkerpeksag berkerpeksag commented Jun 16, 2017

    I personally never need this feature before so I will add Michael and Robert to nosy list to take their opinions.

    @grzgrzgrz3
    Copy link
    Mannequin

    @grzgrzgrz3 grzgrzgrz3 mannequin commented Jun 21, 2017

    Existing mock implementation already has that feature. Mock attributes can be limited with spec attribute.

    >>> inner_m = Mock(spec=["method2"], **{"method2.return_value": 1})
    >>> m = Mock(spec=["method1"], **{"method1.return_value": inner_m})
    >>> 
    >>> m.method1().method2()
    1
    >>> 
    >>> m.method1().attr
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python3.5/unittest/mock.py", line 580, in __getattr__
        raise AttributeError("Mock object has no attribute %r" % name)
    AttributeError: Mock object has no attribute 'attr'

    @mariocj89
    Copy link
    Mannequin Author

    @mariocj89 mariocj89 mannequin commented Jun 21, 2017

    Whilst I agree that using spec can be used for a similar purpose and I did not know about being able to do nested definitions via the arguments (the **{"method1.return_value": 1}, really cool!) I find the idea of allowing users to halt the mock generation really useful. It is much less disruptive and feels more natural.

    Compare:
    >>> inner_m = Mock(spec=["method2"], **{"method2.return_value": 1})
    >>> m = Mock(spec=["method1"], **{"method1.return_value": inner_m})
    
    with: 
    >>> m = mock.Mock()
    >>> m.method1().method2() = 1
    >>> mock.seal(m)

    In brief, seal allows users to just add the method to their existing workflow where they use generic mocks. Moreover, it is extremely user friendly, many of the developers that struggle with the mocking module found seal really helpful.

    @voidspace
    Copy link
    Contributor

    @voidspace voidspace commented Jun 21, 2017

    I don't see what this buys over spec and autospec. I'd be inclined to close it without a compelling use case beyond what is already supported.

    @vstinner
    Copy link
    Member

    @vstinner vstinner commented Jun 22, 2017

    I don't see what this buys over spec and autospec. I'd be inclined to close it without a compelling use case beyond what is already supported.

    I proposed to Mario to open an issue since I like his API. Even if "sealing" mocks is unlikely to be the most common case, when you need it, I prefer his API over the specs thing which reminds me bad time with mox. I prefer the declarative Python-like API, rather than Mock(spec=["method2"], **{"method2.return_value": 1}).

    But yeah, technically specs and sealing seems similar. It's just another way to describe a mock. Since I prefer sealing, I would like to allow users to choose between specs and sealing.

    @voidspace
    Copy link
    Contributor

    @voidspace voidspace commented Jun 23, 2017

    Note that you can use an object as the parameter to the spec argument rather than just a list of attributes.

    Hmmm... I'm not totally opposed to the addition of a "seal_mock" method (optionally with a recurse boolean for child mocks) being added to the Mock/MagicMock classes. It's quite a nice API. I don't like the idea of an additional Mock class for this.

    @vstinner
    Copy link
    Member

    @vstinner vstinner commented Jun 23, 2017

    I don't like the idea of an additional Mock class for this.

    Hum, in the current implementation, it's an enhancement of the Mock class, no more a new class.

    @vstinner
    Copy link
    Member

    @vstinner vstinner commented Sep 4, 2017

    I will merge the PR this week, the PR now LGTM.

    @vstinner
    Copy link
    Member

    @vstinner vstinner commented Oct 17, 2017

    New changeset 552be9d by Victor Stinner (Mario Corchero) in branch 'master':
    bpo-30541: Add new method to seal mocks (GH61923)
    552be9d

    @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.7 stdlib type-feature
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants