ni / nimi-python

Python bindings for NI Modular Instrument drivers.
Other
112 stars 84 forks source link

coverage rcs need to be updated #2015

Closed ni-jfitzger closed 11 months ago

ni-jfitzger commented 12 months ago

Description of issue

tools/coverage_system_tests.rc controls what files are examined for coverage when system tests are run tools/coverage_unit_tests.rc controls what files are examined for coverage when unit tests are run

Both of these likely need updating. We don't report any system test session.py coverage. I checked the niscope/session.py coverage with system tests using pytest-cov and it was 93%, but that's going unreported.

These files are a big part of the reason why our reported test coverage is so low (63.51% for PRs) in codecov.

ni-jfitzger commented 12 months ago

I suspect that much of the configuration of these files was set with the intent to eventually gate submission of PRs on test coverage %.

It seems like part of the problem is that coverage is split between system tests and unit tests, making a high threshold impossible for some modules. The codecov status badge in our READMEs looks at all reported coverage, though, as far as I know.

The poorly done omission of files in each .rc is making our coverage look worse than it is and omitting coverage of some pretty important files.

Here is the coverage of niscope, according to pytest-cov: image

Some of the files should certainly be omitted. I don't think we need to care about the generated pb2 files. Eliminating those will cause our coverage to shoot way up. __init__.py is probably covered by unit tests, if anything?

I think we should always report system test coverage for the interpreters and session.py.

ni-jfitzger commented 12 months ago

I don't know if .rc files allow for comments, but if they do, I'd like to add them.

ni-jfitzger commented 11 months ago

It looks like comments are supported: https://coverage.readthedocs.io/en/latest/config.html

bkeryan commented 11 months ago

It seems like part of the problem is that coverage is split between system tests and unit tests, making a high threshold impossible for some modules.

This is solvable:

https://github.com/ni/nidaqmx-python/blob/master/.github/workflows/report_test_results.yml does something similar, but with test results instead of coverage results.

I don't think we need to care about the generated pb2 files. Eliminating those will cause our coverage to shoot way up.

Good point. FYI:

Most of the lines in *_pb2.py are only used for the pure-Python implementation of protobuf, and the default implementation nowadays is the upb C implementation.

*_pb2_grpc.py defines a stub, a servicer, and an "experiemental API", and you probably only use the stub.