python-tuf icon indicating copy to clipboard operation
python-tuf copied to clipboard

Review coverage report about non-tested statements

Open MVrachev opened this issue 3 years ago • 4 comments

Description of issue or feature request: I run 1coverage` locally to find out at how many places do we miss unit test coverage. We are pretty good right now with coverage of 98% or we have 1138 total statements and only 16 are not tested. I thought it could be useful to review those 16 lines and decide if testing is needed.

PS: If you want to run coverage locally you can from inside the tests directory by running:

python -m coverage run aggregate_tests.py
python -m coverage html
<BROSWER OF CHOICE> htmlcov/index.html

MVrachev avatar Feb 08 '22 15:02 MVrachev

hi @MVrachev i am looking forward to work on this project can you drop some insights and assign it to me?

jalajk24 avatar Sep 09 '22 20:09 jalajk24

Hi, welcome, sorry to have kept you waiting: I'll assign, but there's no need to wait for that in future: if you just leave a note saying you're looking at a bug that's totally fine too.

Running those commands martin listed, will let you review the code annotated by coverage:

  • It may be useful to review the missing (red) and partially run (yellow) lines, see if we could test any of them better
  • deciding whether testing them is useful may require understanding the context -- many of them are not worth the trouble but there may also be cases where we've just missed a test. Feel free to ask here or the python-tuf slack channel if you find a case that looks like it maybe should be tested but isn't
  • how to improve the coverage will depend on the case: let's discuss those if you find something

I have not thought this out 100% but one possible improvement might be several cases where we have a if-elif-construct that doesn't test the unwritten else case (see get_roles_for_target() in metadata.py): in many cases we don't test for the else-case (where neither if nor elif is true) because the metadata structure would be in a state that is invalid... It might be more reasonable to explicitly error out if that happens, at least in some cases?

jku avatar Sep 20 '22 09:09 jku

Yes, sorry @jalajk24 that I made you wait for so long...

MVrachev avatar Sep 20 '22 13:09 MVrachev

@jku can you plz guide me how to start this issue it would be really helpful for me

jalajk24 avatar Nov 14 '22 20:11 jalajk24