Travis testing

[TomMaullin]

This PR aims to implement Travis Testing for the SPM html viewer using Docker containers and Octave.

[TomMaullin]

Whilst creating this PR I have come across the following issue. The below command I believe should run the testing suite for the SPM viewer in a docker container on my machine.

sudo docker run -ti -v /home/tommaullin/Documents/Repos/nidmresults-spmhtml/test:/test cmaumet/octave-spm octave --no-window-system --eval "addpath('/test'); runTest()"

This appears to run the code as required. However, Octave throws an error saying it doesn't have the import function (which is used to load the matlab unittesting framework library).

"error: the import function is not yet implemented in Octave"

I found a fix on stack overflow here but this has given no luck as it has just transformed the error to this:

"warning: function /test/import.m shadows a core library function error: invalid package name: matlab.unittest.TestSuite"

This PR may be put on hold until the next Octave release.

[gllmflndn]

The matlab.unittest framework is not available in Octave and it would be a rather big task to implement it in its entirety. From what I can see in the tests here, you don't use many of its functionalities though so I wonder whether we could have a small workaround or use something like MOdox instead.

[TomMaullin]

Ah I see - Thank you for clarifying this! Do you know of any example repositories than currently contain a MOdox-based testing suite?

[cmaumet]

Thank you @gllmflndn!

@TomMaullin: Does this help: https://github.com/MOdox/MOdox#use-with-travis-ci-and-shippable ?

[TomMaullin]

I am a little confused by this foo.m example in the readme.md. I will see if I can find further examples online and let you know how it goes!

[gllmflndn]

That's because I gave you a link to the wrong project, have a look at MOxUnit instead. Sorry!

If you were OK to use function-based unit tests instead of class-based ones, we could otherwise write a little wrapper for Octave.

[TomMaullin]

Ah - no worries! Yes, this is exactly what I was looking for - Thank you!

I see no reason why these tests couldn't be function based instead of class based... I believe, if I remember correctly, I actually wrote these as functions first before plugging them into the matlab unit testing framework. I think this is why they barely make use of the class structure.

@cmaumet what do you think of this suggestion? I quite like the look of the MOxUnit tests and I don't believe it would be too much work to convert the current tests into this format.

[cmaumet]

@TomMaullin: I would vote for the option that is the easiest to implement so pretty much up to you :)

[TomMaullin]

Okay great! I'll give the MOxUnit tests a go and let you know how they go!

[TomMaullin]

I have been using Octave a bit more recently and realized that the NIDMResults viewer itself does not run on Octave (many of the functions it uses are not currently implemented in Octave). As the tests themselves run the viewer, I am unsure we will be able to test this repository on Travis using dockers with Octave unless the viewer itself is also Octave-friendly.

An example of the issue is given by running the command below in the cmaumet\octave-spm docker container, having installed all of the relevant packages:

octave --no-window-system --eval "addpath(genpath('/nidmresults-spmhtml/'));nidm_results_display('testDataSets2.m')"

This results in:

Please read <http://www.octave.org/missing.html> to learn how you can
contribute missing functionality.
error: 'contains' undefined near line 17 column 8

I at first tried making temporary files to replace the missing functions but I think there are quite a lot of missing functions. I'm honestly not sure how to move on from here. @cmaumet I was wondering, how did you get SPM running in Octave? Is SPM octave-friendly?

[gllmflndn]

The NIDM-Results export from SPM should work with Octave 4.4. Could you list the missing functions that you are trying to use? You mention contains (endsWith would be more adequate here but is not yet available either) but it is only available since R2016b so you would anyway be better off checking the file extension with fileparts/strncmp.

[cmaumet]

@TomMaullin: can you rebase on master in order to include the octave-compatibility updates from #38? And then we can check if the tests pass?

[cmaumet]

Thanks @TomMaullin! I've just enabled Travis CI for this repository and created a dummy commit to trigger the tests.

[cmaumet]

This looks good to me! Thanks for making this work!! I only have a couple of comments of external files you have included in the repo.

Regarding your question on keeping the test suite that is using unittest for Matlab testing, could you let us know what is your preferred option?

[cmaumet]

@TomMaullin: is it normal that you have included this chi2inv.m file in the version-control?

[TomMaullin]

Ah apologies... I thought I had removed this! I'll remove it now!

[cmaumet]

Could we patch on-the-fly as suggested by Guillaume in #38 (comment)?

[TomMaullin]

Oh! Right! I had misunderstood here, yes I will change this! I thought this was intended to go in the docker image eventually.

[gllmflndn]

Hopefully it's only temporary as SPM dev includes the workaround and there is a patch submitted for Octave.

[TomMaullin]

Hi @cmaumet ,

My preference would be to keep both versions of the testing framework, both MOxUnit and matlab unittesting framework. The reason for this is that MOxUnit works on Travis whereas matlab unittesting framework does not, but while developing and making changes locally I find the matlab unittesting framework tests are much faster and convenient than MOxUnit as they involve no setup or bug fixes.

[gllmflndn]

Great! The Octave statistics package should probably be installed through pkg() but do you actually need it? From what I can see, you could just use the spm_*pdf.m and spm_*cdf.m functions from SPM.

[TomMaullin]

Hi @gllmflndn , Oh right! Yes, I hadn't thought of that - that would make life a lot easier! I'll give this a go and let you know how smoothly it goes on Octave!

[TomMaullin]

Quick update: The SPM functions worked and the octave-statistics dependency has been removed. Thank you for the suggestion @gllmflndn !

[cmaumet]

Can we remove this from .gitignore, now that the code is not depending anymore on octave-statistics?

[TomMaullin]

Whoops, yes this is now done!

[cmaumet]

@TomMaullin: Great!! And let's keep both versions of the tests then! Could you document alternative ways to run the tests using: 2/ Octave and 3/ Octave+docker in the README? Also, do we need to put back runTest.m for testing in Matlab? I currently get the following error:

>> addpath(fullfile(fileparts(which('nidm_results_display')), 'test'))
>> runTest();
Undefined function or variable 'runTest'.

Once this is done, +1 to merge.

[TomMaullin]

@cmaumet , great! I will do this!

Yes, currently the Matlab unittests have been removed so the original way of calling the tests no longer works. But I will add them back in and document everything!

[TomMaullin]

Hi @cmaumet ,

Both testing suites are now available and documented in the readme.md. Please let me know if you have any further feedback.

[cmaumet]

Thanks! +1 to merge.