Coverage measurement for Cython code

[oscarbenjamin]

Currently spin test supports --coverage and --gcov arguments for measuring coverage of python or C code respectively. For python-flint the one outstanding item in moving from setuptools to spin/meson is coverage measurement of Cython code.

A meson project needs to have some sort of configuration option for coverage of Cython e.g.:

if get_option('coverage')
   add_project_arguments('-X', 'linetrace=True', language : 'cython')
   add_project_arguments('-DCYTHON_TRACE=1', language : 'c')
endif

In python-flint's case we need this separately from b_coverage=true and spin's --gcov option because we explicitly do not want to measure coverage of C code (which is all Cython-generated).

I'm not sure what the best approach would be for spin to implement spin test --coverage-cython given that there is not a standard meson option for this.

Maybe spin could configure the CYTHON_TRACE and linetrace directly?

Currently it seems that Cython's coverage plugin does not work in a spin build-install directory which I think is because the plugin tries to look for Cython source files adjacent to the C files. Possibly that is something that needs to be fixed in Cython or maybe it is possible for spin to tell coverage where to find the source files.

[oscarbenjamin]

I've made a repo that demonstrates how to get working Cython coverage with setuptools but also shows what does not work with spin:
https://github.com/oscarbenjamin/cython_coverage_demo

At least some change in Cython's coverage plugin is needed.

Also possibly the way that spin/meson generate the .c files results in them containing the wrong path.

[stefanv]

@oscarbenjamin I'd like to see Cython coverage work. I'm a bit tied up for the next few days, but will take a look after that, if you haven't found a solution by then already.

[oscarbenjamin]

The primary blocker to Cython code coverage with spin/meson is that changes are needed in Cython's coverage plugin. I opened a related Cython issue cython/cython#6186.

[oscarbenjamin]

I have described in cython/cython#6186 (comment) how I think we can make coverage work with Cython for a spin/meson-based project.

I think that the proper solution is that spin/meson needs to make something like a json file that tells the coverage plugin where all the files are.

[oscarbenjamin]

I have opened a Cython PR with the minimal fix needed to make this work which is just to ensure that the trace events from Cython are usable: cython/cython#6341.

In combination with that I managed to get coverage measurement working for python-flint but it involved a significant rewrite of Cython's coverage plugin: flintlib/python-flint#188.

I'm wondering now what makes sense to do with that rewritten plugin. It does not share much code with the original Cython coverage plugin and would not be compatible with it. Much of its code is meson-specific because it is a lot easier to write the plugin if you can just use the build system to figure out where all the files are. Cython's current plugin has a bit of a mess of (setuptools-based) heuristics to try to locate all the files and it would be difficult to mix meson-specific code in there I think. It is possible that a clean break from that plugin is just better for everyone because I doubt I could change it easily without breaking workflows.

Potentially it would make sense for the rewritten plugin to be part of spin though. I'm sure it would need further work to generalise it beyond python-flint's needs. One limitation for now is that I've hard-coded the use of a src-layout. Obviously for spin that would need to be configurable somehow.

[stefanv]

Having looked at your PR, it looks like the plugin can easily be shipped stand-alone, and it probably doesn't make sense to limit its use to only projects that use spin?

[oscarbenjamin]

it probably doesn't make sense to limit its use to only projects that use spin?

Maybe. It is very specific to meson in the way that it is written.

I'm still simplifying it a bit now but I doubt that I am personally going to turn it into something sufficiently general that other projects could use it unless they are very similar to python-flint (spin, meson, src-layout, particular types of cython files...). It has already been a bunch of work and it would take some more to make it more widely usable.

I'm very happy for anyone to fork it under whatever license though. (Although I still need to remove the bits that came from Apache-licensed Cython first...)

[oscarbenjamin]

Actually I've simplified it down enough now that it looks plausible to include in Cython itself if they don't mind having some meson-specific code. I guess ultimately that would be the best thing.

[oscarbenjamin]

One thing that does need changing in spin actually is that it ignores .coveragerc. With my setup this works:

spin run -- coverage run -m flint.test -tq

However this does not:

spin test --coverage

I think it fails because it is not looking at my .coveragerc and so does not activate any coverage plugin.

[run]
plugins = coverage_plugin

[stefanv]

We call coverage via pytest:

        pytest_args = [
            *pytest_args,
            "--cov-report=term",
            f"--cov-report=html:{coverage_dir}",
            f"--cov={package}",
        ]

pytest --help says:

  --cov-config=PATH     Config file for coverage. Default: .coveragerc

So, not sure why it's not picking up your .coveragerc :/

[oscarbenjamin]

So, not sure why it's not picking up your .coveragerc :/

I think it is because the working directory was changed. I can run pytest like this:

$ export PYTHONPATH="/home/oscar/current/active/python-flint/build-install/usr/local/lib/python3.11/site-packages"
$ PYTHONPATH=$(pwd):$PYTHONPATH pytest --pyargs flint.test --cov=flint

Apparently I have to include $(pwd) in PYTHONPATH this way to pick up coverage_plugin which is a module in the current directory. If I don't then I get:

$ pytest --pyargs flint.test --cov=flint
Traceback (most recent call last):
...
ModuleNotFoundError: No module named 'coverage_plugin'

In other words pytest saw the .coveragerc but did not allow the coverage_plugin module to be imported because . is not in sys.path.

When I use spin test it claims that it uses cd before invoking pytest:

$ spin test --coverage
Invoking `build` prior to running tests:
$ meson compile -C build
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: /home/oscar/.pyenv/shims/ninja -C /home/oscar/current/active/python-flint/build
ninja: Entering directory `/home/oscar/current/active/python-flint/build'
ninja: no work to do.
$ meson install --only-changed -C build --destdir ../build-install
$ export PYTHONPATH="/home/oscar/current/active/python-flint/build-install/usr/local/lib/python3.11/site-packages:/home/oscar/current/active/python-flint/build-install/usr/local/lib/python3.11/site-packages"
$ export PYTHONPATH="/home/oscar/current/active/python-flint/build-install/usr/local/lib/python3.11/site-packages"
$ /home/oscar/.pyenv/versions/3.11.3/envs/python-flint-3.11/bin/python3.11 -P -c 'import flint'
Removing `/home/oscar/current/active/python-flint/build/coverage/`
$ cd /home/oscar/current/active/python-flint/build-install/usr/local/lib/python3.11/site-packages
$ /home/oscar/.pyenv/versions/3.11.3/envs/python-flint-3.11/bin/python3.11 -P -m pytest --import-mode=importlib --pyargs flint --cov-report=term --cov-report=html:/home/oscar/current/active/python-flint/build/coverage/ --cov=flint
======================================== test session starts ========================================
platform linux -- Python 3.11.3, pytest-7.4.3, pluggy-1.2.0
rootdir: /home/oscar/current/active/python-flint
plugins: hypothesis-6.84.2, cov-4.1.0, split-0.8.1, timeout-2.1.0, doctestplus-1.0.0, xdist-3.3.1
collected 52 items                                                                                  

flint/test/test_all.py ....................................................                   [100%]/
...
home/oscar/.pyenv/versions/3.11.3/envs/python-flint-3.11/lib/python3.11/site-packages/coverage/report_core.py:115: CoverageWarning: Couldn't parse '/home/oscar/current/active/python-flint/build-install/usr/local/lib/python3.11/site-packages/flint/flint_base/flint_base.pyx': No source for code: '/home/oscar/current/active/python-flint/build-install/usr/local/lib/python3.11/site-packages/flint/flint_base/flint_base.pyx'. (couldnt-parse)
  coverage._warn(msg, slug="couldnt-parse")
...

There are lots of "coudnt-parse" errors which come from the coverage plugin not being enabled.

I don't know exactly how to interpret the line

cd /home/oscar/current/active/python-flint/build-install/usr/local/lib/python3.11/site-packages

as seen in the spin output. Is it literally changing the working directory or at least setting a new directory for subprocesses?

Either way I assume that the cd is what causes the .coveragerc to be unnoticed.

[stefanv]

Yes, that's exactly it; we try to get away from the source dir to avoid source files being picked up, in a non-editable install.

We could copy certain special files over to the site-packages dir; I don't think it can cause harm. It is a bit "magical", though.