search

cbirdferrer / collatrix

this tool collates the outputs of the MorphoMetriX photogrammetry tool
MIT License
4 stars 2 forks source link

Automated tests are missing #2

Closed devarops closed 4 years ago

devarops commented 4 years ago

There are no automated tests. I suggest using pytest-qt (or Qt Test) in conjunction with GitHub Actions or Travis. You can find more information here:

cbirdferrer commented 4 years ago

Hi! Thank you for this suggestion. We appreciate that automated testing would help improve our workflow. We have added an automated test through GitHub actions for the main function because this is the function that could be edited in the future if MorphoMetriX is updated. We also don’t anticipate the add-on functions being edited given that we intend for contributions to CollatriX to be in the form of new add-on functions.

devarops commented 4 years ago

Thank you for addressing my suggestion. The use of GitHub Actions to run your tests automatically is a notable improvement of your software. Congratulations!

However, one of the most relevant parts of CollatriX is the GUI and, as of now, it is not tested.

I understand the reasons you provided for not testing extensively, and I even agree that those reasons are valid for a personal project. However, for an open-source project, a code coverage of 22% is quite low.

While trying to obtain a code coverage of 100% is impractical, it is customary to aim for anything between 70% and 80%.

Since CollatriX is open-source software, I would encourage you to obtain a code coverage of al least 40%.

To verify the coverage of your code, you could use the command:

pytest --cov-config=.coveragerc --cov=collatrix --cov-report=term-missing --verbose

I encourage you to replace the last line of your .github/workflows/testing.yml file with that command.

Since your tests are in the collatrix/ subdirectory, you will have to add the file .coveragerc to the root of the project with the following content:

[run]
omit = collatrix/tests/*

Finally, don't forget to add pytest-cov to line 26 of .github/workflows/testing.yml.

cbirdferrer commented 4 years ago

Hi! I appreciate the feedback. I've added the .coveragerc file and updated the testing.yml. I've looked into using pytest-qt to test the GUI functions. However, I've struggled to find good documentation or examples for using pytest-qt and monkey patch on the functions I use. I was wondering if you knew of any helpful examples or resources for testing QInputDialog.getItem, QInputDialog.getText, and QInputDialog.getOpenFileName?

KristinaRiemer commented 4 years ago

@evaristor I don't know if you have any suggestions for testing these functions for @cbirdferrer?

devarops commented 4 years ago

@cbirdferrer @KristinaRiemer:

This tutorial looks useful since it tests this GUI.

You could also get inspired by these code examples that use QInputDialog:

cbirdferrer commented 4 years ago

Thanks @KristinaRiemer!

Hi @evaristor, thank you for finding those examples! Your comments and suggestions have helped me improve the software and I greatly appreciate that! However, I’m wondering if adding further automated testing is necessary, especially since JOSS considers well-documented manual testing to be acceptable in the review criteria- https://joss.readthedocs.io/en/latest/review_criteria.html#tests

devarops commented 4 years ago

Sounds good. I'll go ahead and close this issue now. It looks good. 👌🏾