Add nightly tests for read api

[dorukozturk]

Enhancement

Add nightly tests for pynwb.

Problem/Use Case

In order to avoid regressions with the read API, check diverse set of nwb files against pynwb dev branch.
I prefer not to slow down our test suite, that is why I am proposing nightly tests.

I am thinking:

1. We upload 5-10 diverse NWB files to a public server (hoping to collect different files from
   different labs to cover as much as possible)
2. Every night a cron job starts and downloads those files
3. Tests the read api against these diverse sample files
4. Reports back the results (it can be a separate badge or email)

Does this sound good?

[bendichter]

What's the protocol if it reveals a bug in read?

[dorukozturk]

@bendichter I can think of 2 cases and they have different solutions. Both cases require making the tests pass.

a) We unintentionally broke it, that means we have to revert part of the code which breaks the read API for the sake of backwards compatibility.

b) We intentionally change the read API, which means we have to fix the tests to reflect the new read API on tests and bump up the version of pynwb so that people can track breaking changes and pin specific versions in their code so that they don't get surprises.

Is this reasonable?

[oruebel]

Creating support for nightly tests to do longer-running tests sounds good. In addition to read, I think you would also want to have round-trip tests in there, i.e. write a file and read it back.

I'm adding @nclack as they have been looking at doing some of this for the MatNWB MatLab API as well and this issue seems relevant to the round-trip testing between PyNWB and MatNWB to test that the PyNWB can read files written with MatNWB and vice-versa.

@dorukozturk what implementation plan do you have in mind for this?

[jcfr]

nightly tests to do longer-running tests sounds good

This could be setup as TravisCI cron job. Here is an example of such job.

The script on the travis repo, could also trigger a parameterized CircleCi build (example of such trigger here)

[dorukozturk]

@oruebel Adding round trip tests sounds great. That kind of end to end testing is very valuable for
stability.

Circleci has a feature for setting up cron jobs (https://circleci.com/docs/2.0/workflows/#scheduling-a-workflow). I am planning to add a new workflow to our current circle config file which will trigger
certain tests at midnight. @oruebel How does that sound?

@jcfr do you see any obstacles with this approach?

[oruebel]

Just FYI, here the pull request with the test-suite for the MatLab API: NeurodataWithoutBorders/matnwb#23

I'm mainly adding this to make sure you are aware of what is going on that end. We probably want to run these tests separately for PyNWB as well, but I think these kind of tests may potentially be a good basis for testing compatibility between the APIs.

@jcfr @dorukozturk do you have any particular tests ready or already in mind for this?

[dorukozturk]

I have 3 kinds of tests in mind:

1. Find sample files (hopefully from different labs) that are diverse enough so that I can just do simple .read() on them.
2. Round trip or end to end tests (I might need some help from you guys here)
3. Test code on documentation page. We have nice examples there but time to time they don't work so people cannot follow along. I want to cover those examples too.

I would be more than happy to discuss/add/remove items from the list above. If all of the above sounds good, I will work on the PR.

[oruebel]

Test code on documentation page

@ajtritt is working on making the code as sphinx-galleries which should make this easier https://github.com/NeurodataWithoutBorders/pynwb/tree/enh/sphinx_gallery

Round trip or end to end tests

I think the base setup for this is here https://github.com/NeurodataWithoutBorders/pynwb/tree/dev/tests/integration

Find sample files

Generally speaking, I think most files can be "made" small, i.e., the bulk of the data is going to be in large arrays, e.g., for time series. For most tests, it should not matter whether you are writing a 5x100 or 100x1000000 array so you can just trim down these arrays for testing.

[jcfr]

Circleci has a feature for setting up cron jobs

Neat. 👍

do you see any obstacles with this approach?

Leveraging CircleCI for this kind of test makes perfect sense . The CircleCI caching mechanism can also be very useful to speedup testing by avoiding downloading of python packages and/or data.

[mgrauer]

Would it make sense to also run these longer tests on merges to master?

[bendichter]

This only makes sense for tests that will take a long time. For quick tests, it would be better to add them to the integration tests that are already in place. Do you have specific tests in mind that take a long time to run?

I have a conversion script for publicly available data from the Buzsaki Lab that takes a few minutes. I'm happy to contribute that as soon as I work out the last few remaining bugs. We could run the conversion script and then run read on it to test round trip.

3. The code on the documentation page is currently run as an integration test. It failed recently in a way that wasn't tested: the file could be written without error, but produced an error when trying to read it. To fix this, we just need to add a file.read() before we delete it. I think that addition of read would not add much time to the test, and I'd prefer to catch these things at the integration test if we can.

[bendichter]

@mgrauer We currently don't have a master branch, but I would be in favor of creating one and, as you suggest, running comprehensive integration tests on merge

[mgrauer]

Sorry @bendichter, I meant dev branch in this repo. I believe we are currently doing this pre/post merge split of tests, where some CIs (OS platforms) are run on PRs and all are run after merges into dev. These tests (if they are long running) could also be added to the post merge tests.

@dorukozturk Can you confirm or correct me here?