Automated testing of notebooks

[birdage]

myself and a few others (@bobfrat,@daf) have been discussing automated testing of notebooks through travis ci. The testing scope would be pretty limited, i.e just running the notebooks to make sure that no errors are found. This would help when merging, as the merger could see straight away that there might be an issue.

[ocefpaf]

I like the simplicity of this idea. It will help to evaluate the PRs. Because we already save the .py file for every notebook, it would be a matter of running them and catching the Exceptions. A long term plan would be to add docstrings to the functions in utilities.py and run them as well.

However, we should not rely completely on travis-ci green light for merging. Even with the testing scheme in place we should send the nbviewer url with every PR to compare results and check the outputs (specially when updating an existing notebook).

PS: For the .travis.yml we will need to use conda instead of the python modules available in travis-ci virtual machine.

Here is an example on how to do that: https://github.com/SciTools/iris/blob/master/.travis.yml

[ocefpaf]

I tried to implement something along those lines. (Warning: it is very hackish! We can do better!!)

The testing module: https://github.com/ocefpaf/secoora/blob/master/notebooks/test_notebooks.py

And the .travis.yml config: https://github.com/ocefpaf/secoora/blob/master/.travis.yml

By implementing this I realized that travis will return several (if not all) failures by hanging up. That is due to the heavy download that happens is almost all the notebooks in system-test: https://travis-ci.org/ocefpaf/secoora/builds/32639783

Any ideas? Split the tests by mocking the download and use saved files for the second part?

[rsignell-usgs]

It looks like Travis times out because it didn't receive output for 10 minutes. So the inundation notebook takes more than 10 minutes to run? It shouldn't. We should be smarter about eliminating models that don't need to be tested. And other notebooks run faster, right? Can set at least get it testing the ones that are fast?

[birdage]

@rsignell-usgs i think we should remove some of the extreme notebooks from that list as most of them have large data access.

[rsignell-usgs]

Yes, or maybe @ocefpaf can figure out how to eliminate some of the models that get accessed but never used (perhaps by being more clever in the script)

[ocefpaf]

@birdage For now I think we should add all the notebooks to get some idea of which ones will fail. Later on we can start skipping, non-silently though, those notebooks that are hanging-up.

@rsignell-usgs The SECOORA inundation notebook minimizes* the download by using the KDTree for searching the data. The download only happens if data are found within the requested distance.

There are still some download waste when the data are located over land**. The script does not know if it is land or water until it downloads and checks the data. Maybe I could create a land mask and eliminate all point inside that before hand... Not sure what is the best approach here.

Here is the log file with the download times for each model:

No data near* takes just a few seconds.

land** takes > 1 minute.

[jkupiec]

Folks, Is this an enhancement that we can reasonably expect to be implemented within the next 6 weeks or so, or is it a larger effort that must wait for a future project?

[birdage]

@ocefpaf i think that is a good starting point yes, just wanted to point out that different notebooks may have different response times when it comes to collecting data.

[birdage]

@jkupiec good question, i think it would be a "nice to have" from an integration stand point, and may help closer to the deadline as we try to get things squared away so we are not scrambling to fix errors from a merge. But on the flip side if we are correctly testing notebooks before merging this wouldnt be as much as an issue. my 2 cents.

[ocefpaf]

@birdage Can you setup a travis account for system-test (if one does not already exist )? I can send a PR based on what I do for SECOORA if you want.

[ocefpaf]

I just sent a PR (#152) with 3 (actually 2) tests.

• test_system-test_rules.py check if the .py, pip-requirement.txt and conda-requirement.txt files exist for each notebook found.
• test_notebooks.py will run all notebooks found using runipy and a common environment installed by the .travis config file. The reasons I chose runipy are:
  • We can save the html output for visualization.
  • runipy creates a log of the run that can be inspected.
  • We can run the ipython magics present in the notebooks that would fail in a regular python call.
• test_environments.py is not implemented! It is just a skeleton for a test that would create a virtualenv or a conda virtual env based on the requirement file for the notebook.

I do not know how travis will "react" to this. As soon as someone creates the Travis account we can start playing with this and fix the broken stuff.

[birdage]

@ocefpaf i spoke to @daf that there isnt really a need to create a travis account, it attaches to the github account , i dont have admin access to the SIT repo so cant flip the switch to turn travis on, but i have it on for my fork of the SIT repo https://travis-ci.org/birdage/system-test

[ocefpaf]

By "creating the travis account" I meant exactly that: attach the ioos/system-test and flip the switch.

@birdage I did the same for my branch. But if the main ioos/system-test has travis on we will see the test status for every PR.

[birdage]

@ocefpaf indeed! we need someone with github admin access on that repo to flip the switch, unfortunately i dont have it. ill add the badge as well to the read me :+1: @kknee for info

[kwilcox]

Travis seems to be doing something, closing this issue. Further issues with Travis can have their own home.