Add `--gcov-report` flag to `spin test` #159

ganesh-k13 · 2024-01-26T13:30:21Z

Changes

Added --gcov-report flag to spin test

spin test --help                                                                                                    1 ↵ ganesh@ganesh-MS-7B86
Usage: spin test [OPTIONS] [PYTEST_ARGS]...
...
  --gcov-report [html|xml|text|sonarqube]
                                  Generate a coverage report for C Code and exit
...

Testing

Happy Path

» spin test --gcov
...
» spin test --gcov-report html                                                                                    ganesh@ganesh-MS-7B86
Verifying installed packages for generating coverage reports...
$ ninja -C build -t targets all
Generating html coverage report...
$ ninja -C build coverage-html
Coverage report generated successfully and written to /home/ganesh/os/devpy/example_pkg/build/meson-logs/coveragereport/index.html

Failures

No build found

» git clean -xdf                                                                                                           ganesh@ganesh-MS-7B86
Removing .spin/__pycache__/
Removing build-install/
Removing build/
» spin test --generate-gcov-report html                                                                                    ganesh@ganesh-MS-7B86
Verifying installed packages for generating coverage reports...
Error: `build` folder not found, cannot generate coverage reports. Generate coverage artefacts by running `spin test --gcov`

Gcovr, etc not installed

» spin test --gcov-report html                                                                                   ganesh@ganesh-MS-7B86
Verifying installed packages for generating coverage reports...
$ ninja -C build -t targets all
Error: coverage-html is not supported... Ensure the following are installed: Gcovr/GenHTML, lcov and rerun `spin test --gcov`

Notes

ToDo

UT

Misc

resolves #158
continues #146

ganesh-k13 · 2024-01-27T07:45:16Z

.github/workflows/test.sh

+    echo "Yes"
+fi
+
+# TODO: Test other coverage reports


TODO: I'll cover this as a part of #154 to avoid bloat here

spin/cmds/meson.py

stefanv · 2024-01-27T21:03:49Z

Thanks, Ganesh, I've done a brief pass and all looks good. There are a few small tweaks I'd like to make, which I'll do Monday morning.

Instead of generate-gcov-report, perhaps the shorter gcov-report? Happy to make that tweak myself, just wanted to make sure you don't object.

ganesh-k13 · 2024-01-28T04:37:05Z

Thanks for the review @stefanv , I have renamed the flag to --gcov-report and updated the description.

stefanv · 2024-01-30T00:29:44Z

spin/cmds/meson.py

+    if f"coverage-{coverage_type.value}" not in p.stdout.decode("ascii"):
+        raise click.ClickException(
+            f"coverage-{coverage_type.value} is not supported... "
+            f"Ensure the following are installed: {', '.join(requirements[coverage_type])} "


This error is raised when spin build; spin test --gcov-report. Needs to ensure that build was run with --gcov.

Yep, shall I add a dependency to call spin build --gcov in case it's missing?

Ah ok, I get the problem. We need to check for coverage artifacts. Let me add it.

Added checks and UT in f2c21eb

stefanv · 2024-01-30T00:48:44Z

After looking at this some more, I'm wondering if the gcov-report option is associated with the right command. It seems unrelated to the test options, and rely on the build having been performed with --gcov.

Also, doing spin build, and then spin build --gcov isn't enough. You need spin build --clean --gcov.

So, I'd suggest moving --gcov-report to build, setting html as the default, automatically enabling --gcov when --gcov-report is set, and also detecting when the existing build wasn't run with --gcov (you can check the build/build.ninja file for coverage-* commands).

rgommers · 2024-01-30T10:23:54Z

Also, doing spin build, and then spin build --gcov isn't enough. You need spin build --clean --gcov.

I'll note that this isn't specific to --gcov, it applies to anything that translates to a compile/link flag. --werror and --asan, which SciPy's dev.py has any would also be useful for spin, are good examples.

So, I'd suggest moving --gcov-report to build, setting html as the default, automatically enabling --gcov when --gcov-report is set

I'm not sure I understand that suggestion, or maybe I'm missing your point. You typically build from scratch once, and run tests many times when you're developing. You don't want a coverage report every time you run tests, so it shouldn't be done automatically. But conceptually, you want the coverage report for the tests you've just run I think, through a "give me the gcov coverage report". So it could be integrated with test, or possibly standalone - but it doesn't belong with build.

stefanv · 2024-01-30T15:49:32Z

That was my original understanding, but then I couldn't see where the gcov outputs are generated by pytest. I clearly missed it, so I'll take another look.

rgommers · 2024-01-30T16:35:00Z

That was my original understanding, but then I couldn't see where the gcov outputs are generated by pytest. I clearly missed it, so I'll take another look.

It's a separate command, unrelated to pytest. That command does need to be run after the test run of interest. E.g., see https://github.com/scipy/scipy/blob/maintenance/1.10.x/runtests.py#L510-L533.

spin/cmds/meson.py

spin/tests/test_build_cmds.py

ganesh-k13 · 2024-03-09T04:22:46Z

noxfile.py

 @nox.session
 def test(session: nox.Session) -> None:
-    session.install(".", "pytest", "build", "meson-python", "ninja")
+    session.install(".", "pytest", "build", "meson-python", "ninja", "gcovr")


@stefanv / @jarrodmillman , should this be part of a test_requirements.yaml for depandabot to pick up and auto update or this is ok?

Co-authored-by: Jarrod Millman <[email protected]>

stefanv · 2024-04-30T23:56:46Z

@ganesh-k13 Sorry for the delay working on this. I've reworked the logic a bit, as detailed in the commit message of 135279f. Take a look, and see if you approve of what I've done?

- Like the existing coverage option, `spin test --gcov` is now enough to generate gcov reports. - The format of the report is set with `--gcov-format`. - Coverage builds are triggered when `--gcov` is added (necessary to generate the coverage report), but no rebuild is done when the flag is removed (too expensive).

ganesh-k13

Thanks, looks good! I cannot approve as I'm the author, but looks good to go. I'll port this to NumPy and hopefully enable code cov reports in the CI soon.

jarrodmillman added the type: Enhancement New feature or request label Jan 26, 2024

ganesh-k13 commented Jan 27, 2024

View reviewed changes

spin/cmds/meson.py Outdated Show resolved Hide resolved

ganesh-k13 force-pushed the enh_158_coverage_report branch 4 times, most recently from 4299ecd to 3b5fdfc Compare January 27, 2024 07:55

ganesh-k13 marked this pull request as ready for review January 27, 2024 07:57

stefanv mentioned this pull request Jan 27, 2024

Add a pytest based testing framework and tests for build #160

Merged

4 tasks

ganesh-k13 changed the title ~~Added --generate-gcov-report flag to spin test~~ Added --gcov-report flag to spin test Jan 28, 2024

ganesh-k13 changed the title ~~Added --gcov-report flag to spin test~~ Add --gcov-report flag to spin test Jan 28, 2024

stefanv reviewed Jan 30, 2024

View reviewed changes

jarrodmillman reviewed Feb 9, 2024

View reviewed changes

spin/cmds/meson.py Outdated Show resolved Hide resolved

ganesh-k13 force-pushed the enh_158_coverage_report branch from 0d18f0c to b57bc87 Compare February 29, 2024 06:33

ganesh-k13 commented Feb 29, 2024

View reviewed changes

spin/tests/test_build_cmds.py Show resolved Hide resolved

ganesh-k13 force-pushed the enh_158_coverage_report branch from 81ea2da to 9749de0 Compare March 9, 2024 04:19

ganesh-k13 commented Mar 9, 2024

View reviewed changes

ganesh-k13 requested a review from stefanv March 9, 2024 04:22

ganesh-k13 and others added 6 commits April 30, 2024 15:53

ENH: Added --generate-gcov-report flag to spin test

d4db9c1

TST: Added testcases for html coverage reports

42e0ad9

MAINT: Rename flag to gcov-report

73c9b9d

MAINT: Fix doc string for gcov-report

78bde64

Co-authored-by: Jarrod Millman <[email protected]>

TST: Added UT for gcov-report

e00e56b

ENH, TST: Added checks for missing debug files

0c06a53

stefanv force-pushed the enh_158_coverage_report branch from f2c21eb to 389193d Compare April 30, 2024 23:55

stefanv force-pushed the enh_158_coverage_report branch from 389193d to 135279f Compare April 30, 2024 23:58

stefanv added 2 commits April 30, 2024 16:59

Fix test command in README

d62c834

Fix tests, now that coverage build is triggered automatically

24690f2

jarrodmillman approved these changes May 1, 2024

View reviewed changes

ganesh-k13 commented May 1, 2024

View reviewed changes

jarrodmillman merged commit 00b9c31 into scientific-python:main May 1, 2024

jarrodmillman added this to the 0.9 milestone May 1, 2024

ganesh-k13 mentioned this pull request Dec 5, 2024

ENH: Add generate-lcov-html flag #258

Closed

1 task

Add --gcov-report flag to spin test #159

Add --gcov-report flag to spin test #159

Uh oh!

Conversation

ganesh-k13 commented Jan 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Testing

Happy Path

Failures

Notes

ToDo

Misc

Uh oh!

ganesh-k13 Jan 27, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stefanv commented Jan 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ganesh-k13 commented Jan 28, 2024

Uh oh!

stefanv Jan 30, 2024

Choose a reason for hiding this comment

Uh oh!

ganesh-k13 Feb 29, 2024

Choose a reason for hiding this comment

Uh oh!

ganesh-k13 Mar 9, 2024

Choose a reason for hiding this comment

Uh oh!

ganesh-k13 Mar 9, 2024

Choose a reason for hiding this comment

Uh oh!

stefanv commented Jan 30, 2024

Uh oh!

rgommers commented Jan 30, 2024

Uh oh!

stefanv commented Jan 30, 2024

Uh oh!

rgommers commented Jan 30, 2024

Uh oh!

Uh oh!

Uh oh!

ganesh-k13 Mar 9, 2024

Choose a reason for hiding this comment

Uh oh!

stefanv commented Apr 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ganesh-k13 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Add `--gcov-report` flag to `spin test` #159

Add `--gcov-report` flag to `spin test` #159

ganesh-k13 commented Jan 26, 2024 •

edited

Loading

stefanv commented Jan 27, 2024 •

edited

Loading

stefanv commented Apr 30, 2024 •

edited

Loading