Open Chilipp opened 3 years ago
hey @kuadrat! thanks for adding these tests to the software! Just a few small suggestions:
I would not add pytest
to the requirements.txt
file as it is not necessary for using the software in production. Instead you should add it using the extras_require
parameter in your setup.py
, i.e. something like
setup(
...,
extras_require={"tests": ["pytest>=6.2.2", "pytest-qt>=3.3.0"]},
)
or, even better, create a tox.ini
file that lists the test commands and dependencies such that you only have to run tox
to run all tests.
Anyway, these are just recommendations and no blocking issues for the review in https://github.com/openjournals/joss-reviews/issues/2969. I check the review items and leave this issue here open, but feel free to close it if you want.
It is me who has to thank you, @Chilipp, for the very useful suggestions (also with the other issues) and +1 for making me aware of tox
!
Now I definitely want to set things up with tox
. In my first tries with it today, however, it turned out to not be straightforward, likely due to Qt binding version conflicts, so I'll focus on the other issues for now ;)
I could not find any automated tests or documented steps to verify the functionality of the software, but this is a requirement for JOSS
I think these tests are crucial for a software like yours. As you are extracting and visualizing data, you should include them in your unit tests. From my personal experience, I strongly recommend to use
pytest
in combination withpytest-qt
Disclaimer
this issue is part of the review for JOSS in https://github.com/openjournals/joss-reviews/issues/2969