Papermill: Get code coverage to ~90%

[rgbkrk]

We can do it and anyone can help. 😄

I've tagged this as Hacktoberfest friendly, feel free to ask questions.

Requirements

Must have knowledge:

• Python

Optional (you can learn it):

• py.test

Issue Creation

It's ok if people want to create issues for each of the untested areas then go ahead and tackle them

[achou11]

Would love to attempt this help out with this if I can! I don't have any experience with testing or code coverage but would like to learn.

[rgbkrk]

You're on the hook for some example notebooks in vdom too! :wink:

To get started, I recommend cloning this repository and looking at the current tests in papermill/tests. You can see how the tests run by looking at Travis' configuration here: https://github.com/nteract/papermill/blob/master/.travis.yml#L17

Once you've installed pytest yourself (pip install -U pytest), you should also be able to run the test suite locally on your clone of this repository. If you run into snags, let us know.

[achou11]

Ok thanks! Some questions before I dive into it:

• Do I have to worry about what Python version I'm using? I've been running into some problems with 2.7 and I usually stick to 3 anyways.

• Should I have a virtual environment set up? I already have all of the dependencies installed so I didn't see the need to do this, but it seems like good practice.

• Am I focusing on writing tests or fixing to match tests? Or does it depend on the situation? Seems like a silly question but like I said, I don't really have much experience with this aspect 😅 (feel free to point me to any resources that would answer this)

[rgbkrk]

Feel free to stick with Python 3, you can always let Travis do the checking to make sure it works on both Python 2 and Python 3.

Should I have a virtual environment set up? I already have all of the dependencies installed so I didn't see the need to do this, but it seems like good practice.

It helps so you insulate yourself from changes (even your own). It's not a strict requirement though. I'm running papermill from master on my main python install.

Am I focusing on writing tests or fixing to match tests?

For this, you'll be writing tests, in a similar style to the current ones. If you look at papermill's codecov you can see which files have the least coverage, then click in to files. I'd start off with api.py and see if there's something you could exercise in a few lines of python.

For instance, the _get_notebook_outputs function is not tested in any way:

It takes a notebook object, created either by reading a file or by creating a notebook. Here's a dummy notebook you could use to test with:

from nbformat.v4 import (
    new_notebook, new_code_cell, new_markdown_cell, new_output
)

nb = new_notebook(
    cells=[
        new_code_cell('test', outputs=[
            nbformat.v4.new_output(
                output_type='display_data',
                data={},
                metadata={
                    'papermill': {
                        'name': 'test'
                    }
                }
            )
        ])
    ]
)

That creates the same in-memory object that _get_notebook_outputs uses. You want to write a test that uses that nb to test the validity of _get_notebook_outputs, similar to this:

from nbformat.v4 import (
    new_notebook, new_code_cell, new_markdown_cell, new_output
)

def test_get_notebook_outputs():

    output = nbformat.v4.new_output(output_type='display_data', data={},
        metadata={ 'papermill': { 'name': 'test' } })
    nb = new_notebook(
        cells=[
            new_code_cell('test', outputs=[ output ])
        ]
    )

    assert _get_notebook_outputs(nb) == { 'test': output }

That's at least a start. Looking over the function, I'd test what happens when a cell has zero outputs, no outputs with a name metadata, when they have the papermill metadata but no name, and notebooks with markdown cells (since they have no outputs). In the process, you'll probably see other edge cases to cover as well as possibly uncovering bugs. You never know until you try!

[rgbkrk]

One other thing -- there's code here that's a bit harder to test because you'll need to write mocks. When getting started with testing it's probably hard to figure out which to go after first and what it takes to write mocks. Generally, anything involving writing to disk or making network requests is going to need a mock.

pytest provides guidance for mocks which I'd use to mock the IPython.display.display function for some of the untested in papermill/api.py.

[aaronmak]

That's really helpful advice. I'll give it a go!

[willingc]

Thanks @aaronmak. Feel free to ping me with questions too.

[Madhu94]

@aaronmak You can also ask questions in the slack channel. If you haven't joined please request for invite

[aaronmak]

@Madhu94 Just joined! Would be sure to ping you guys if there's anything. You guys have been a great help so far!

[MSeal]

A bit late, but we got there :cocktail:

Ok I cheated and changed the title to 90% to match the milestone, but still