ahip, hip, async tests and coverage

[pquentin]

We have an issue with code coverage now that we're starting to test our async backends.

Our source code layout looks like this in git:

/src/ahip/poolmanager.py
/test/async/test_poolmanager.py
/test/util.py

Before running tests, we:

• install using unasync, which gives two packages:

  .nox/test-3-8/lib/python3.8/site-packages/ahip/poolmanager.py
  .nox/test-3-8/lib/python3.8/site-packages/hip/poolmanager.py
  

• run unasync on tests:

  /test/async/test_poolmanager.py
  /test/sync/test_poolmanager.py
  /test/util.py
  

• and run the tests on the installed packages

I guess ideally we'd like to see coverage both on ahip/poolmanager.py and hip/poolmanager.py, but hip/poolmanager.py isn't on git, for reasons explained in #149.

In #149 we mention that mapping sync code back to async code fixes the coverage issue. It's not that simple though, because we want to keep our hard-earned coverage during the transition to async tests.

By configuring pytest-cov with --cov=hip and --cov=ahip, an unmodified XML coverage report contains those lines:

<class name="poolmanager.py" filename=".nox/test-3-8/lib/python3.8/site-packages/ahip/poolmanager.py" complexity="0" line-rate="0.732" branch-rate="0">
<class name="poolmanager.py" filename=".nox/test-3-8/lib/python3.8/site-packages/hip/poolmanager.py" complexity="0" line-rate="1" branch-rate="0">

We can't just remove the synchronous version because it's the only one that's fully covered, and we can't just remove the async version because some of the code only runs in ahip, not hip. The more general solution is to merge the two files. I guess it's feasible in noxfile.py where we already parse coverage.xml. The way I'd do it:

• replace .nox/test-3-8/lib/python3.8/site-packages/ with src/ in all filenames attributes, possibly do the same things in packages names
• merge src/hip/... into src/ahip/... by keeping the covered lines from the sync version, then remove the src/hip coverage info

When we send the coverage file to codecov, we can pretend that only src/ahip exists, which is good because it's the only thing that codecov can handle.

Thoughts?

[njsmith]

That all sounds right to me.

It's possible we might be able to convince coverage to do the combining for us, using coverage combine? It's not really what it's designed for, but it does know how to remap paths and merge coverage, so it's pretty close... https://coverage.readthedocs.io/en/stable/cmd.html#combining-data-files

Another stupid trick that might work is to split the coverage into two files, and pass them both to codecov?

Or actually... it might even make sense to run codecov twice, passing different "flags". That way in the codecov UI it would show combined coverage by default, but we could hit a toggle to show only coverage that originated in async mode or sync mode.

[sethmlarson]

Maybe saving two separate files, rewriting the file paths hip -> ahip in the sync coverage file and then running coverage combine? Extra steps but I think it should work.

[pquentin]

Thanks for the suggestions, they helped a lot!

coverage combine works with coverage.py SQLite files, not the XML reports. What I'm currently doing is asking coverage to cover both hip and ahip, then generate a hip report and a ahip report, and send that to codecov with flags: master...pquentin:async-coverage

But wait.py is still marked as only 80% covered, and I don't understand why. I also see some comments covered, which suggest a bug somewhere before codecov. The next part of the plan is to look at all files sent to codecov and see what's wrong with them.