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

@@ -3,5 +3,5 @@

 @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
17 checks passed

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

Draft

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

ganesh-k13 commented Jan 26, 2024 •

edited

Loading

ganesh-k13 Jan 27, 2024

stefanv commented Jan 27, 2024 •

edited

Loading

ganesh-k13 commented Jan 28, 2024

stefanv Jan 30, 2024

ganesh-k13 Feb 29, 2024

ganesh-k13 Mar 9, 2024

ganesh-k13 Mar 9, 2024

stefanv commented Jan 30, 2024

rgommers commented Jan 30, 2024

stefanv commented Jan 30, 2024

rgommers commented Jan 30, 2024

ganesh-k13 Mar 9, 2024

stefanv commented Apr 30, 2024 •

edited

Loading

ganesh-k13 left a comment

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

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

Conversation

ganesh-k13 commented Jan 26, 2024 • edited Loading

Changes

Testing

Happy Path

Failures

Notes

ToDo

Misc

ganesh-k13 Jan 27, 2024

Choose a reason for hiding this comment

stefanv commented Jan 27, 2024 • edited Loading

ganesh-k13 commented Jan 28, 2024

stefanv Jan 30, 2024

Choose a reason for hiding this comment

ganesh-k13 Feb 29, 2024

Choose a reason for hiding this comment

ganesh-k13 Mar 9, 2024

Choose a reason for hiding this comment

ganesh-k13 Mar 9, 2024

Choose a reason for hiding this comment

stefanv commented Jan 30, 2024

rgommers commented Jan 30, 2024

stefanv commented Jan 30, 2024

rgommers commented Jan 30, 2024

ganesh-k13 Mar 9, 2024

Choose a reason for hiding this comment

stefanv commented Apr 30, 2024 • edited Loading

ganesh-k13 left a comment

Choose a reason for hiding this comment

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