RF: centralize invocation of coverage within run_coverage and use sys.executable -m coverage

[yarikoptic]

Motivation

• duplication is evil
• some systems might have it installed as python3-coverage not as coverage (Debian)
• executable "coverage" might use shebang pointing to another python than used here

Originally crafted for Debian package

Note that locally I have two tests there failing while testing 2.5.0 release even without this diff

❯ python -m pytest -s -v -k TestValidateCLI
================================================================ test session starts =================================================================
platform linux -- Python 3.11.7, pytest-7.4.3, pluggy-1.3.0 -- /home/yoh/deb/gits/pkg-exppsy/pynwb/venvs/dev3.11/bin/python
cachedir: .pytest_cache
rootdir: /home/yoh/deb/gits/pkg-exppsy/pynwb
collected 603 items / 591 deselected / 12 selected                                                                                                   

tests/validation/test_validate.py::TestValidateCLI::test_validate_file_cached PASSED
tests/validation/test_validate.py::TestValidateCLI::test_validate_file_cached_bad_ns PASSED
tests/validation/test_validate.py::TestValidateCLI::test_validate_file_cached_core PASSED
tests/validation/test_validate.py::TestValidateCLI::test_validate_file_cached_extension PASSED
tests/validation/test_validate.py::TestValidateCLI::test_validate_file_cached_extension_pass_ns PASSED
tests/validation/test_validate.py::TestValidateCLI::test_validate_file_cached_hdmf_common PASSED
tests/validation/test_validate.py::TestValidateCLI::test_validate_file_cached_ignore PASSED
tests/validation/test_validate.py::TestValidateCLI::test_validate_file_invalid PASSED
tests/validation/test_validate.py::TestValidateCLI::test_validate_file_list_namespaces_core PASSED
tests/validation/test_validate.py::TestValidateCLI::test_validate_file_list_namespaces_extension PASSED
tests/validation/test_validate.py::TestValidateCLI::test_validate_file_no_cache FAILED
tests/validation/test_validate.py::TestValidateCLI::test_validate_file_no_cache_bad_ns FAILED

====================================================================== FAILURES ======================================================================
____________________________________________________ TestValidateCLI.test_validate_file_no_cache _____________________________________________________

self = <tests.validation.test_validate.TestValidateCLI testMethod=test_validate_file_no_cache>

    def test_validate_file_no_cache(self):
        """Test that validating a file with no cached spec against the core namespace succeeds."""
        result = subprocess.run(["coverage", "run", "-p", "-m", "pynwb.validate",
                                 "tests/back_compat/1.0.2_nwbfile.nwb"], capture_output=True)
    
        stderr_regex = re.compile(
            r".*UserWarning: No cached namespaces found in tests/back_compat/1\.0\.2_nwbfile\.nwb\s*"
            r"warnings.warn\(msg\)\s*"
            r"The file tests/back_compat/1\.0\.2_nwbfile\.nwb has no cached namespace information\. "
            r"Falling back to PyNWB namespace information\.\s*"
        )
>       self.assertRegex(result.stderr.decode('utf-8'), stderr_regex)
E       AssertionError: Regex didn't match: '.*UserWarning: No cached namespaces found in tests/back_compat/1\\.0\\.2_nwbfile\\.nwb\\s*warnings.warn\\(msg\\)\\s*The file tests/back_compat/1\\.0\\.2_nwbfile\\.nwb has no cached namespace information\\. Falling back to PyNWB namespace information\\.\\s*' not found in 'The file tests/back_compat/1.0.2_nwbfile.nwb has no cached namespace information. Falling back to PyNWB namespace information.\n'

tests/validation/test_validate.py:37: AssertionError
_________________________________________________ TestValidateCLI.test_validate_file_no_cache_bad_ns _________________________________________________

self = <tests.validation.test_validate.TestValidateCLI testMethod=test_validate_file_no_cache_bad_ns>

    def test_validate_file_no_cache_bad_ns(self):
        """Test that validating a file with no cached spec against a specified, unknown namespace fails."""
        result = subprocess.run(["coverage", "run", "-p", "-m", "pynwb.validate", "tests/back_compat/1.0.2_nwbfile.nwb",
                                 "--ns", "notfound"], capture_output=True)
    
        stderr_regex = re.compile(
            r".*UserWarning: No cached namespaces found in tests/back_compat/1\.0\.2_nwbfile\.nwb\s*"
            r"warnings.warn\(msg\)\s*"
            r"The file tests/back_compat/1\.0\.2_nwbfile\.nwb has no cached namespace information\. "
            r"Falling back to PyNWB namespace information\.\s*"
            r"The namespace 'notfound' could not be found in PyNWB namespace information as only "
            r"\['core'\] is present\.\s*"
        )
>       self.assertRegex(result.stderr.decode('utf-8'), stderr_regex)
E       AssertionError: Regex didn't match: ".*UserWarning: No cached namespaces found in tests/back_compat/1\\.0\\.2_nwbfile\\.nwb\\s*warnings.warn\\(msg\\)\\s*The file tests/back_compat/1\\.0\\.2_nwbfile\\.nwb has no cached namespace information\\. Falling back to PyNWB namespace information\\.\\s*The namespace 'notfound' could not be found in PyNWB namespace information as only \\['core'\\] is present\\.\\s*" not found in "The file tests/back_compat/1.0.2_nwbfile.nwb has no cached namespace information. Falling back to PyNWB namespace information.\nThe namespace 'notfound' could not be found in PyNWB namespace information as only ['core'] is present.\n"

tests/validation/test_validate.py:57: AssertionError
============================================================== short test summary info ===============================================================
FAILED tests/validation/test_validate.py::TestValidateCLI::test_validate_file_no_cache - AssertionError: Regex didn't match: '.*UserWarning: No cached namespaces found in tests/back_compat/1\\.0\\.2_nwbfile\\.nwb\\s*warnings.warn\\(ms...
FAILED tests/validation/test_validate.py::TestValidateCLI::test_validate_file_no_cache_bad_ns - AssertionError: Regex didn't match: ".*UserWarning: No cached namespaces found in tests/back_compat/1\\.0\\.2_nwbfile\\.nwb\\s*warnings.warn\\(ms...
=================================================== 2 failed, 10 passed, 591 deselected in 32.49s ====================================================
python -m pytest -s -v -k TestValidateCLI  42.32s user 35.34s system 237% cpu 32.669 total

edit 1: doesn't for you it generate bunch of .coverage.* files you have to cleanup manually? IMHO should be avoided (didn't check if avoidable or should just be cleaned up later)

Checklist

• Did you update CHANGELOG.md with your changes?
• Have you checked our Contributing document?
• Have you ensured the PR clearly describes the problem and the solution?
• Is your contribution compliant with our coding style? This can be checked running flake8 from the source directory.
• Have you checked to ensure that there aren't other open Pull Requests for the same change?
• Have you included the relevant issue number using "Fix #XXX" notation where XXX is the issue number? By including "Fix #XXX" you allow GitHub to close issue #XXX when the PR is merged.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6498000) 91.99% compared to head (73d71b6) 91.99%.

Additional details and impacted files

@@           Coverage Diff           @@
##              dev    #1811   +/-   ##
=======================================
  Coverage   91.99%   91.99%           
=======================================
  Files          27       27           
  Lines        2623     2623           
  Branches      685      685           
=======================================
  Hits         2413     2413           
  Misses        138      138           
  Partials       72       72           

Flag	Coverage Δ
integration	71.10% <ø> (ø)
unit	83.68% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rly]

Note that locally I have two tests there failing while testing 2.5.0 release even without this diff

This test failure was introduced when we recently released a new version of HDMF. The test was fixed in #1778. However, the fix has not yet been released. We will release this soon but not during the holiday break.

edit 1: doesn't for you it generate bunch of .coverage.* files you have to cleanup manually? IMHO should be avoided (didn't check if avoidable or should just be cleaned up later)

Yes, it does. I know it is terribly ugly, but I have not yet found a better solution to test command line

[yarikoptic]

Yes, it does. I know it is terribly ugly, but I have not yet found a better solution to test command line

what are you trying to achieve? to gather coverage from running pynwb based scripts?

If so: we had (have? ;)) similar problem in datalad where we do have command line tools which we wanted coverage of while invoking from command line. Our solution

• have a folder https://github.com/datalad/datalad/tree/HEAD/tools/coverage-bin where command line tools actually are just symlinks to http://github.com/datalad/datalad/blob/HEAD/tools/coverage-bin/datalad which modifies invocation of the original command line tool (original name of the symlink) to run through python -m coverage and collect coverage report under /tmp/coverages/
• in the CI we just modify PATH:

❯ git grep PATH.*coverage-bin
.appveyor.yml:  - sh: PATH=$PWD/../tools/coverage-bin:$PATH python -m pytest -c ../tox.ini -n 2 -s -v -m "not (turtle)" --cov=datalad --pyargs ${DTS}
.travis.yml:    PATH=$PWD/../tools/coverage-bin:$PATH
tools/coverage-bin/datalad:export PYTHONPATH="$PWD/../tools/coverage-bin/"

• and at the end of the CI we combine all those reports into the main one: https://github.com/datalad/datalad/blob/HEAD/.appveyor.yml#L350 and then submit to codecov

[rly]

Yes, it does. I know it is terribly ugly, but I have not yet found a better solution to test command line

what are you trying to achieve? to gather coverage from running pynwb based scripts?

Yes, exactly.

in the CI we just modify PATH

Clever! I'll explore this, but yes, we want to do basically the same thing. Thanks for the tip!