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

the tb.patch will only allow strings to be returned from mocks #72

Closed
manniche opened this issue Nov 11, 2020 · 6 comments
Closed

the tb.patch will only allow strings to be returned from mocks #72

manniche opened this issue Nov 11, 2020 · 6 comments

Comments

@manniche
Copy link

manniche commented Nov 11, 2020

I have a notebook that calls some library function and gets a complex object returned. Since the object ultimately comes from a database call, I want to mock this such that the notebook gets a synthetic object instead. I have tried with the following pattern:

from testbook import testbook
from collections import namedtuple

@testbook('./doc/examples/notebooks/tools/01-CreateFromObjectNo.ipynb')
def test_get_object(tb):
    mock_object = namedtuple("dataobject",["name", "description"])
    mock_return__function = lambda x: mock_object("Some Name", "Some Description")
    
    with tb.patch("library.dataobjects.DataObject.retrieve", mock_return_function):
        tb.execute_cell([2, 4])

The notebook cell 2 handles imports and the notebook cell 4 looks something like:

dataobject = DataObject.retrieve("DO-320")
print(dataobject.name)
print(dataobject.description)

The test the fails with:

AttributeError: 'str' object has no attribute 'name'

Am I missing something in the documentation or can we only return strings with the wrapped patch?

@rohitsanj
Copy link
Member

rohitsanj commented Nov 11, 2020

Hey! First of all, thank you for using testbook! :)

Here are a few comments about the code snippet provided:

  1. Please make sure to execute the cells with all necessary imports before referencing library.dataobjects.DataObject.retrieve in the tb.patch contextmanager.

  2. Our implementation of tb.patch takes in the target as the only positional argument, please pass in the function created with an explicit key word argument such as side_effect. Or in your specific case, it looks like you can directly pass in return_value and skip the lambda.

  3. Instead of printing and asserting against the output text of the cell, you can directly access the notebook variables through the tb.ref API - this is the USP of testbook over other similar tools. Hence you can move the test code from the notebook into the unit testing function.

  4. patch instead of patch_func

Here is the suggested code snippet:

from testbook import testbook
from collections import namedtuple

@testbook('./doc/examples/notebooks/tools/01-CreateFromObjectNo.ipynb')
def test_get_object(tb):
    mock_object = namedtuple("dataobject", ["name", "description"])
    with tb.patch(
        "library.dataobjects.DataObject.retrieve",
        return_value=mock_object("Some Name", "Some Description"),
    ):
        tb.execute_cell([2, 4])

        # obtain reference to the mocked `retrieve` function
        retrieve = tb.ref("DataObject.retrieve")
        dataobject = retrieve("DO-320")

        assert dataobject.name == "Some Name"
        assert dataobject.description == "Some Description"

@rohitsanj
Copy link
Member

Also, please note that tb.execute_cell([2, 4]) will execute the cells with indices 2 and 4, and it will NOT execute cells from 2 to 4 i.e 2, 3, 4. I just wanted to clarify that. 👍

In case you do want to execute cells from 2 to 4 (inclusive), you would need to pass in a slice object as follows:

tb.execute_cell(slice(2, 5)) # end is exclusive, hence it will execute until 5 - 1 = 4

@manniche
Copy link
Author

manniche commented Nov 11, 2020

Thanks for providing testbook, which looks extremely useful for my case of testing notebooks. The patch_func was my attempt at trying to hack a bit at the codebase. I will edit the text to reflect the base case.

And yes, the intention was to only execute cell 2 and 4, since cell 3 contains markdown. Cell 2 contains the import statements and cell 4 the logic that retrieves the object and queries its attributes.

The point is not to test the return values of name or description, but simply to have this object available in the notebook such that we avoid hitting the database and also get reliably repeatable conditions for the test. Currently the test fails because the notebook code receives a str object, where it expects and object that has the attributes name and description.

@manniche
Copy link
Author

After trying to experiment with the source code I have the understanding now that we cannot carry arbitrary custom defined types through the translators.py (since that would warrant an import of said type).

Instead I utilized the possibility of injecting code into the notebook to ensure arbitrary types could be mocked via the unittest.mock.patch method (in the present case, but the technique is easily generalizable):

from testbook import testbook
from collections import namedtuple

@testbook('./doc/examples/notebooks/tools/01-CreateFromObjectNo.ipynb')
def test_get_object(tb):
    tb.execute_cell(2)  # execute cell with import statements

    tb.inject(
        """
        from unittest.mock import patch
        from collections import namedtuple
        mock_object = namedtuple("dataobject", ["name", "description"])
        p1 = patch(
            "library.dataobjects.DataObject.retrieve",
            return_value=mock_object("Some Name", "Some Description")
        )
        p1.start()
        """
    )

    tb.execute_cell(4)  # execute cell with business logic

    tb.inject(
        """
        p1.stop()
        """
    )
    
    assert tb.cell_output_text(4) == "Some Name\n\nSome Description"

@rohitsanj
Copy link
Member

After trying to experiment with the source code I have the understanding now that we cannot carry arbitrary custom defined types through the translators.py (since that would warrant an import of said type).

That's right, which is why we have built the tb.ref method which acts like a dummy object whose attributes when accessed, are converted into code and injected and then the output of that is retrieved back into the unit test, and if that output is JSON serializable (strings, numbers, dict etc) then it would bring back the value itself, if not, it would create another reference object. You can read more about that here.

Instead I utilized the possibility of injecting code into the notebook to ensure arbitrary types could be mocked via the unittest.mock.patch method (in the present case, but the technique is easily generalizable):

No worries, I hope this approach solved the problem.

You can take a look at the reference for inject for more flexibility while injecting. Few arguments you can do pass to the inject function:

  • pass in run=False (default is True) to not execute the cell as soon as it is injected
  • pass in pop=True (default is False) to pop the cell off after execution, this is useful in cases where you would want to execute the entire notebook (or range of cells) again but not including the injected cell.
  • pass in before or after arguments to inject the cells in a particular location in the notebook. Useful when used with run=False.

Note that inject appends a cell and runs it by default.


Coming back to the original issue of :

AttributeError: 'str' object has no attribute 'name'

I am not too sure how that could have happened, but please let me know if this issue still persisted after the changes I had recommended. If not, may I go ahead and close this issue?

PS: Please drop an email to mail@rohitsanjay.com so that I may invite you to nteract's Slack workspace.

@manniche
Copy link
Author

manniche commented Nov 23, 2020

Thanks for the reply, the inject function arguments are very helpful. The generated documentation found at the link you provided is somewhat broken relative to the code. But reading off the code it makes perfect sense.

As to the 'str' object has no attribute 'name' it was because the namedtuple I tried to send through the patch method was converted into a string (by the translator class) before injected into the notebook. In the notebook I tried to access it via the (arbitrary) .name and .description methods, which the str object does not have. The types I want the notebook to receive are not JSON serializable by fiat and my point in #72 (comment) is that writing custom serializers in the nteract code for things like this is not feasible.

TLDR; great tool, documentation could use a brush-up, and using the unittest.mock.patch method directly solved the problem for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants