Add CI / improve testing

[junghans]

Related to openjournals/joss-reviews#3653

The README only refers to one test sisyphe.test_sisyphe(), which takes a long time, only tests some functionality and needs to be run manually.

It would be good to add CI, e.g. GitHub action to repo and run these tests at every push.

Testing python packages is pretty simple, see https://docs.github.com/en/actions/guides/building-and-testing-python

[antoinediez]

Thanks for your feedback.

The function sisyphe.test_sisyphe() is designed to test the core functionalities of the package on a "real-world example". As a consequence, it is indeed computationally demanding, but in the other hand, I chose a model which is theoretically well understood so that it is possible to check the result of the test with accuracy.

In the coming days, I will nevertheless add CI to the package to test more functionalities on smaller examples. Unfortunately, the Github hosted machines do not have a GPU so the tests will be limited to much simpler, small scale cases.

[junghans]

Yeah, I ran sisyphe.test_sisyphe() on my laptop (without GPU) and it took a long time there, too. Not great for a quick test if the package works ;-)

[antoinediez]

I added the quick test module sisyphe.test.quick_test. The various test functions check that the core functionalities work and run small scale simulations (taken form the example gallery) to ensure that the code runs without error. These tests are run automatically at every push as suggested by @junghans. I also updated the installation page of the documentation to warn the user that the more advanced test function sisyphe.test_sisyphe() may take a long time.

Please tell me if the addition of this module answers your comment or if you would like to see something else. I won't close this issue until both of you (@junghans and @lorenzo-rovigatti) tick the "Automated tests" item on the review page.

[junghans]

Great job!

[lorenzo-rovigatti]

Thanks to @junghans for opening this issue and @antoinediez for providing a solution. Unfortunately I'm having some issues with it. I have installed sisyphe by cloning the repo and using pip3 install . in the repo's folder. I'm using Ubuntu, Python 3.8.10, pytest 6.2.5.

The following snippet

import pytest
from sisyphe.test import quick_test
retcode = pytest.main(quick_test.__file__)

results in

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/lorenzo/.local/lib/python3.8/site-packages/_pytest/config/__init__.py", line 143, in main
    config = _prepareconfig(args, plugins)
  File "/home/lorenzo/.local/lib/python3.8/site-packages/_pytest/config/__init__.py", line 307, in _prepareconfig
    raise TypeError(msg.format(args, type(args)))
TypeError: `args` parameter expected to be a list of strings, got: '/home/lorenzo/SCRATCH/JOSS_REVIEWS/Sisyphe/sisyphe/test/quick_test.py' (type: <class 'str'>)

I never used Pytest but looking at the error I tried to fix it with retcode = pytest.main([quick_test.__file__, ]) and the test procedure starts.

Unfortunately this results in quite a few FileExistsError errors. They seem trivial but I cannot get around them. I attach the output to this post.

test_output.txt

[antoinediez]

Hi @lorenzo-rovigatti !

Thanks for correcting retcode = pytest.main(quick_test.__file__, ) into retcode = pytest.main([quick_test.__file__, ]). The previous syntax is depreciated and does not work with the latest version of Pytest. I will update the documentation.

For the second (and more critical) issue, I think that it comes from PyKeOps. What does the following snippet output on your machine ?

import pykeops
pykeops.clean_pykeops()         
pykeops.test_torch_bindings()

If it does not work then, I think that it is related to this issue getkeops/keops#142. This should be solved by upgrading Cmake to Cmake>=3.18.

To check that, I tried the following: on a Google Colab session, typing

!pip install sisyphe
import pytest
from sisyphe.test import quick_test
retcode = pytest.main([quick_test.__file__,])

gives exactly the same error as yours. However, by typing first

!pip install pykeops[colab]

this will upgrade Cmake to Cmake 3.18 before installing PyKeOps and the tests then run as expected.

[lorenzo-rovigatti]

Yes, the snippet does not work. I'll try to update cmake and see if that solves the issue!

[lorenzo-rovigatti]

Updating cmake does solve the issue. Maybe you want to add this requirement somewhere in the documentation. Either than that I think you mark this issue as resolved!

[antoinediez]

Great thank you!
I did not add Cmake in the requirements because it is already a requirement of PyKeOps. It is likely that the authors will add Cmake>=3.18 as a requirement in a future release (or find another way to solve the issue). In the meantime, I will add a note in the documentation to warn the user that upgrading Cmake may be necessary.