Codecoverage tool fails easily; some lines accidentally covered

[rosesyrett]

In a recent PR https://github.com/bluesky/ophyd/pull/1148 The codecoverage patch job was failing, although all the lines added in the PR had been tested.

The coverage on master for ophyd/ophyd/areadetector/paths.py is currently 91.97%. However, on the PR this same file has a lower coverage, although the PR did not change any of the lines pertaining to these tests or the source code. In fact, the PR concerns itself with ophyd v2 ,and the test coverage drop is for ophyd v1 functionality.

On further inspection, the last latest ophyd release had a codecoverage report with the same code coverage of this file as the PR's code (seen here: https://app.codecov.io/gh/bluesky/ophyd/commit/51679551aab7798424124509d48f7a006e73ca15/blob/ophyd/areadetector/paths.py). It seems like whether or not lines 73-80 get covered in the codecov report is entirely random, and looking at the code it makes sense; this bit of code will only get called if a signal.set call doesn't immediately line up with the value one is trying to set it with. i.e. if for some reason setting the signal takes a bit of time, this block of code will get called.

So, this issue is to propose two fixes:

1. Add an actual test for these lines of code, so that the patch change for the above PR passes,
2. Introduce tolerances for code coverage (+/- 0.1%) within which the jobs will still pass. Considering the sheer volume of tests involved, this is not the first instance where someone changing bits of ophyd code has codecov failing even though they've covered all their new lines of codes in tests.

[callumforrester]

Agree with the fixes, but do we know why this is happening? I don't think I've seen it on other repositories.

[rosesyrett]

The percentage decrease in patch for https://github.com/bluesky/ophyd/pull/1148 is, in my opinion, totally random. I'm sure if I re-ran the CI on master a few times I'd get one CI job where (if coverage were also rerun, all jobs on that PR also) the patch change would be 0% for that PR.

The offending lines are these:

    signal.put(val)
    deadline = ttime.time() + timeout if timeout is not None else None
    current_value = signal.get()

    while not path_compare(current_value, val, semantics=path_semantics):
        logger.debug(
            "Waiting for %s to be set from %r to %r...", signal.name, current_value, val
        )
        ttime.sleep(poll_time)
        if poll_time < 0.1:
            poll_time *= 2  # logarithmic back-off
        current_value = signal.get()

this is inside ophyd/areadetector/paths.py.

So basically the while loop is never entered if everything works smoothly. If things are a bit slow, however (e.g. runners are a bit slower than usual... takes a while for caput to actually complete) then the while loop might well be entered.

So what we've got is, some lines in ophyd are accidentally covered if things are working slowly. Otherwise, if everything works smoothly, they never get hit. Infact, running the tests locally confirms that these lines are never hit. And it is these exact lines that account for the -0.04% patch change between that PR and master.

[rosesyrett]

Just to confirm this, I'm going to rerun the CI on master a few times... hold on...

edit: am struggling to do this, as really I want to re-run code coverage as well as the tests. As evidence of this idea, have a look at the latest commits into main and see how the code coverage on those lines changes randomly... (despite the fact that those commits had nothing to do with those lines).

e.g. Here's the codecov for 1c8487b, commited on the 26th of June into the master btanch. Notice how areadetector/paths.py is listed as an indirect change. You can inspect this file to see it has not covered the lines I've quoted above.

And here's the codecov for the commit right after it for the Ophyd 1.8 release, 5167955. This time it's not even listed as an indirect change, but the coverage on areadetector/paths.py has magically increased because the lines above are suddenly covered. Probably because a slow runner picked up the test job that produced that coverage report.