JOSS Paper review - code comments about taweret HOT 11 CLOSED

Hello,

No, I wouldn't consider it to be blocking at all; it's just a suggestion. Actually, I find the main advantage is that it allows for non-incremental testing (all tests run, even if some have failed).

I'll wrap up my review soon.

from taweret.

I see that most of the original comment is now resolved. I would recommend refactoring the tests to use a UnitTest object-oriented framework. Have you considered setting up CodeCov or similar to track code coverage of tests?

from taweret.

Is there a reason to use the OO framework over what we have now? We have not considered any code coverage, but will now look into it, thank you for the suggestion.

from taweret.

The main advantages of the OO framework are that (1) test runs are named in the test output, and (2) all tests run and errors are only reported at the end, so any errors along the way are clearly documented and don't stop other tests from running.
For example, see passing tests here: https://github.com/julienmalard/traDSSAT/actions/runs/8135344210/job/22229734167
Since they pass, there isn't that much output, but if any failed the output would be much more detailed. In addition, many IDEs such as PyCharm format the test output by subtest in the test runner console if they're written as OO tests.

from taweret.

So I looked into the pytesting framework, particular for OO, and here they advocate for it as a way of incrementally (i.e., only continuing to test if previous steps succeeded) testing your package, otherwise all the examples they give are primarily function based. At least in terms of success reporting, our reports look the same (comparing the workflow you sent us, with what we currently have). From the pytest documentation, I do see the difference you mention about each test being named.

Our question: Is the suggestion of moving towards an OO framework a blocking suggestion?
We will definitely add CodeCov (still need to look into this).

from taweret.

A minor point on code coverage - it seems to be currently set up to measure the coverage of the test files, and not of the package files themselves (which may explain the near-100% coverage).

from taweret.

I tried to google an example of code coverage testing a full repository, but haven't been able to. Could you point me to an example so that I may understand?

from taweret.

Hello,
Here's an example: https://github.com/reseau-constellation/client-python

As can be seen here, the source files are also included in the coverage report.

from taweret.

@julienmalard
Sorry for the delay, we have implemented the code coverage for the whole package

from taweret.

@julienmalard is it okay to close this issue?

from taweret.

Yes, looks good!

from taweret.