SimVascular / svZeroDSolver

A C++ lumped-parameter solver for blood flow and pressure in hemodynamic networks
https://simvascular.github.io/documentation/rom_simulation.html#0d-solver
Other
7 stars 18 forks source link

Add C++ code coverage #32

Closed mrp089 closed 1 year ago

mrp089 commented 1 year ago

I'll add C++ code coverage to our testing as we did in https://github.com/SimVascular/svFSIplus/commit/9c4598744e955778198eb861f61e1005b04b7f8b

mrp089 commented 1 year ago

First coverage report added in #37. Current overall coverage is

Lines: 82.5%
Functions: 100%

I'll add more tests to cover:

@menon-karthik it seems like there are some untested lines in configreader: https://github.com/StanfordCBCL/svZeroDPlus/blob/18c04c628e30f285e8535a410c638762d81a774e/src/io/configreader.hpp#L121-L125 Is that really untested, or is my framework not catching the interface test?

Apart from that, the missing lines are error handling or debug outputs, so I'm very happy with this result @richterjakob, @menon-karthik!

menon-karthik commented 1 year ago

@mrp089 That's weird! Those lines should be covered in the test case below: https://github.com/StanfordCBCL/svZeroDPlus/blob/18c04c628e30f285e8535a410c638762d81a774e/tests/test_interface/test_01/svzerod_3Dcoupling.json#L2-L6

You might not be catching the interface tests. But I'm confused because you wouldn't get 100% function coverage in that case. Unless that number doesn't include the code in src/interface either.

mrp089 commented 1 year ago

Yup, it's not included... I'll look into it.

mrp089 commented 1 year ago

@menon-karthik I just realized I was never executing your tests during the coverage, only pytest. But now I'm confused about what the interface tests are actually doing.

Are they testing any results? Can we include them in our pytest framework?

Edit: Found the result tests!

menon-karthik commented 1 year ago

@menon-karthik I just realized I was never executing your tests during the coverage, only pytest. But now I'm confused about what the interface tests are actually doing.

The interface tests are meant to test external C++ programs interfacing with svZeroDPlus. So the tests are set up as "external" programs in the sense that they are separately compiled, and they then link to the svZeroDPlus shared library interface and call functions from there. The shared library and the functions that can be called from external programs are all in src/interface.

Are they testing any results? Can we include them in our pytest framework?

Yup, they are testing results for the models specified by the two .json files in tests/test_interface/test_01 and tests/test_interface/test_02. They are also testing other functions in the interface (which aren't simulation "results", but still important things to test) to read/update parameters, read/update the state vector, etc., from external codes.

I'm not sure if they can be included in pytest (at least I don't know how to do that) because they are not python scripts. Unless it is possible to also call C++ programs from pytest? Something else I don't understand is why you get 100% function coverage (here) even though the tests in pytest don't include anything in src/interface. It makes sense that nothing in that directory is tested if pytest is not running the interface tests, but it's also somehow being excluded from the coverage report.

mrp089 commented 1 year ago

Makes sense!

From what I understand now, we're building an interface library and then using the test executables to run it. I think lcov can only track executables (e.g., I can see what's being executed in the main.cpp of the interface tests), not the interface.

Since we don't touch any line in src/interface, it seems like lcov just excluded the whole folder.

I think we have to accept the fact that for now we're not getting test coverage on the interface.

menon-karthik commented 1 year ago

Ah ok. Yeah I think it's fine to not get code coverage of the interface for now. We can revisit this if/when we find a way to include it. The interface has just 10 functions anyway, so it's easy to keep track. They're all being tested currently except increment_time, which is not particularly useful currently but might have use in some future application (it just runs one time step from an external program, instead of a full simulation). If you want, you can print a warning from that function here and call it a day.

menon-karthik commented 1 year ago

Closing with #42