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.

[oscarbenjamin]

Coverage docs say:

A different location for the configuration file can be specified with the --rcfile=FILE command line option or with the COVERAGE_RCFILE environment variable.

So there is an environment variable that can be set for coverage at least.

I'm not sure if it would be better to put these things in pyproject.toml somehow and I'm not sure how coverage gets its configuration when run via pytest.

This picks up the .coveragerc:

PYTHONPATH=$(pwd) COVERAGE_RCFILE=$(pwd)/.coveragerc spin test --coverage

However then my plugin fails:

rout.py:372: CoverageWarning: Disabling plug-in 'coverage_plugin.Plugin' due to an exception:
  Traceback (most recent call last):
    File "/home/oscar/.pyenv/versions/3.11.3/envs/python-flint-3.11/lib/python3.11/site-packages/coverage/inorout.py", line 357, in should_trace
      file_tracer = plugin.file_tracer(canonical)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/home/oscar/current/active/python-flint/coverage_plugin.py", line 116, in file_tracer
      return CyFileTracer(srcpath)
             ^^^^^^^^^^^^^^^^^^^^^
    File "/home/oscar/current/active/python-flint/coverage_plugin.py", line 131, in __init__
      assert (src_dir / srcpath).exists()
  AssertionError
  
    self.warn(f"Disabling plug-in {plugin_name!r} due to an exception:\n{tb}")

I guess this is because coverage generates different paths when the CWD changes and the plugin needs to account for that.

[stefanv]

That would be a better solution. Think you can make your plugin work with that configuration? I so I can add the coverage option whenever .coveragerc is present.

[oscarbenjamin]

Think you can make your plugin work with that configuration?

Maybe. It is not straight-forward though.

The way this works is that the plugin registers itself and then coverage calls the plugin's file_tracer method for each file and then the plugin either returns None or returns a FileTracer which will handle the file.

I just tested and under the configuration:

PYTHONPATH=$(pwd) COVERAGE_RCFILE=$(pwd)/.coveragerc spin test --coverage

the plugin did not even receive the .pyx file paths except for 1 out of about 40. The one that it did receive was:

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

It looks like something in coverage filters out paths (based on CWD somehow) before the plugin gets a chance. I assume that a change in coverage would be needed to prevent this from happening.

[oscarbenjamin]

Actually there is another problem with cding into another directory: how is the plugin supposed to find build.ninja?

[oscarbenjamin]

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.

Note that this problem comes from not using src-layout. It shouldn't be necessary to change the working directory like this because the source files should not be implicitly importable by virtue of cwd.

[stefanv]

That's right; but most scientific Python projects, historically, do not use src layout.

[oscarbenjamin]

I think I'm overextended on this for now but when I find time I will open an issue with coverage (the one project I have not yet discussed this with). I think I have found one or two areas where this can only be made to work fully with changes in coverage itself. Ned has always been very helpful to me in the past but I think this needs quite a lengthy explanation and I need to find the time to get it across clearly.

In the meantime if anyone wants to adapt the coverage plugin from python-flint then feel free to do so and re-license as needed (I have already removed anything nontrivial that came from the Cython codebase).

[stefanv]

Thanks for all your hard work on making Cython debugging feasible!

[oscarbenjamin]

No problem. Also in the meantime we await the outcome of the Cython PR: cython/cython#6341