CI: Restore coverage collection after test reorganization#3006
CI: Restore coverage collection after test reorganization#3006
Conversation
Replace pytest-cov plugin with direct coverage.py invocation to resolve
0% coverage reporting issue that occurred after moving tests into nested
subdirectories.
Changes:
- Update CI workflow to use ============================= test session starts ==============================
platform linux -- Python 3.13.11, pytest-9.0.2, pluggy-1.6.0
rootdir: /root/mesa
configfile: pyproject.toml
plugins: mock-3.15.1, anyio-4.12.0, base-url-2.1.0, playwright-0.7.2, cov-7.0.0, ipywidgets-1.55.1
collected 379 items
tests/discrete_space/test_discrete_space.py ............................ [ 7%]
.. [ 7%]
tests/discrete_space/test_grid.py ...................................... [ 17%]
. [ 18%]
tests/discrete_space/test_hexgrid_torus_validation.py . [ 18%]
tests/examples/test_examples.py .......... [ 21%]
tests/examples/test_examples_viz.py ......... [ 23%]
tests/experimental/test_continuous_space.py ....... [ 25%]
tests/experimental/test_devs.py ..... [ 26%]
tests/experimental/test_mesa_signals.py ..... [ 27%]
tests/experimental/test_meta_agents.py ............... [ 31%]
tests/test_agent.py ...................... [ 37%]
tests/test_batch_run.py ........ [ 39%]
tests/test_datacollector.py ............... [ 43%]
tests/test_import_namespace.py . [ 44%]
tests/test_lifespan.py .. [ 44%]
tests/test_mesa_logging.py .. [ 45%]
tests/test_model.py ........... [ 48%]
tests/test_space.py s................................................... [ 61%]
.............................................. [ 73%]
tests/visualization/test_backends.py .............. [ 77%]
tests/visualization/test_components_matplotlib.py ....... [ 79%]
tests/visualization/test_portrayal_components.py ............. [ 82%]
tests/visualization/test_solara_viz.py ......... [ 85%]
tests/visualization/test_solara_viz_updated.py .......... [ 87%]
tests/visualization/test_space_drawer.py .............................. [ 95%]
tests/visualization/test_space_renderer.py ................ [100%]
=============================== warnings summary ===============================
tests/visualization/test_space_renderer.py::test_property_layer_style_instance
/root/mesa/tests/visualization/test_space_renderer.py:200: PendingDeprecationWarning: Passing propertylayer_portrayal to draw_propertylayer() is deprecated. Use setup_propertylayer(propertylayer_portrayal) before calling draw_propertylayer().
sr.draw_propertylayer(style)
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
================================ tests coverage ================================
_______________ coverage: platform linux, python 3.13.11-final-0 _______________
Name Stmts Miss Branch BrPart Cover
-------------------------------------------------------------------------------------------------
mesa/__init__.py 14 0 0 0 100%
mesa/agent.py 194 1 78 3 99%
mesa/batchrunner.py 88 2 36 4 95%
mesa/datacollection.py 157 4 78 4 97%
mesa/discrete_space/__init__.py 9 0 0 0 100%
mesa/discrete_space/cell.py 82 1 18 0 99%
mesa/discrete_space/cell_agent.py 52 0 12 0 100%
mesa/discrete_space/cell_collection.py 56 3 18 1 95%
mesa/discrete_space/discrete_space.py 57 3 4 1 93%
mesa/discrete_space/grid.py 125 6 48 5 94%
mesa/discrete_space/network.py 29 0 6 0 100%
mesa/discrete_space/property_layer.py 141 11 44 3 89%
mesa/discrete_space/voronoi.py 144 0 46 1 99%
mesa/examples/__init__.py 11 0 0 0 100%
mesa/examples/advanced/__init__.py 0 0 0 0 100%
mesa/examples/advanced/alliance_formation/agents.py 8 0 0 0 100%
mesa/examples/advanced/alliance_formation/app.py 23 10 0 0 57%
mesa/examples/advanced/alliance_formation/model.py 74 2 34 4 94%
mesa/examples/advanced/epstein_civil_violence/__init__.py 0 0 0 0 100%
mesa/examples/advanced/epstein_civil_violence/agents.py 68 0 18 1 99%
mesa/examples/advanced/epstein_civil_violence/app.py 28 5 6 2 79%
mesa/examples/advanced/epstein_civil_violence/model.py 48 9 20 5 76%
mesa/examples/advanced/pd_grid/__init__.py 0 0 0 0 100%
mesa/examples/advanced/pd_grid/agents.py 28 3 6 3 82%
mesa/examples/advanced/pd_grid/app.py 11 0 0 0 100%
mesa/examples/advanced/pd_grid/model.py 31 9 10 3 61%
mesa/examples/advanced/sugarscape_g1mt/__init__.py 0 0 0 0 100%
mesa/examples/advanced/sugarscape_g1mt/agents.py 101 3 22 0 94%
mesa/examples/advanced/sugarscape_g1mt/app.py 20 2 2 0 91%
mesa/examples/advanced/sugarscape_g1mt/model.py 52 5 10 2 85%
mesa/examples/advanced/wolf_sheep/__init__.py 0 0 0 0 100%
mesa/examples/advanced/wolf_sheep/agents.py 60 1 14 1 97%
mesa/examples/advanced/wolf_sheep/app.py 34 5 10 2 84%
mesa/examples/advanced/wolf_sheep/model.py 34 0 6 2 95%
mesa/examples/basic/__init__.py 0 0 0 0 100%
mesa/examples/basic/boid_flockers/__init__.py 0 0 0 0 100%
mesa/examples/basic/boid_flockers/agents.py 28 0 2 0 100%
mesa/examples/basic/boid_flockers/app.py 22 1 4 1 92%
mesa/examples/basic/boid_flockers/model.py 34 2 4 1 92%
mesa/examples/basic/boltzmann_wealth_model/__init__.py 0 0 0 0 100%
mesa/examples/basic/boltzmann_wealth_model/agents.py 18 0 4 0 100%
mesa/examples/basic/boltzmann_wealth_model/app.py 19 2 0 0 89%
mesa/examples/basic/boltzmann_wealth_model/model.py 22 0 0 0 100%
mesa/examples/basic/boltzmann_wealth_model/st_app.py 47 47 10 0 0%
mesa/examples/basic/conways_game_of_life/__init__.py 0 0 0 0 100%
mesa/examples/basic/conways_game_of_life/agents.py 31 2 6 0 95%
mesa/examples/basic/conways_game_of_life/app.py 16 3 0 0 81%
mesa/examples/basic/conways_game_of_life/model.py 13 0 2 0 100%
mesa/examples/basic/conways_game_of_life/st_app.py 40 40 10 0 0%
mesa/examples/basic/schelling/__init__.py 0 0 0 0 100%
mesa/examples/basic/schelling/agents.py 22 0 6 0 100%
mesa/examples/basic/schelling/app.py 24 1 6 0 97%
mesa/examples/basic/schelling/model.py 24 0 4 0 100%
mesa/examples/basic/virus_on_network/__init__.py 0 0 0 0 100%
mesa/examples/basic/virus_on_network/agents.py 34 0 12 0 100%
mesa/examples/basic/virus_on_network/app.py 23 6 0 0 74%
mesa/examples/basic/virus_on_network/model.py 36 0 2 0 100%
mesa/experimental/__init__.py 2 0 0 0 100%
mesa/experimental/continuous_space/__init__.py 3 0 0 0 100%
mesa/experimental/continuous_space/continuous_space.py 112 4 14 1 96%
mesa/experimental/continuous_space/continuous_space_agents.py 45 0 4 0 100%
mesa/experimental/devs/__init__.py 3 0 0 0 100%
mesa/experimental/devs/eventlist.py 78 2 20 1 97%
mesa/experimental/devs/simulator.py 112 6 28 4 93%
mesa/experimental/mesa_signals/__init__.py 3 0 0 0 100%
mesa/experimental/mesa_signals/mesa_signal.py 190 10 66 10 92%
mesa/experimental/mesa_signals/observable_collections.py 41 2 0 0 95%
mesa/experimental/mesa_signals/signals_util.py 19 2 2 0 90%
mesa/experimental/meta_agents/__init__.py 2 0 0 0 100%
mesa/experimental/meta_agents/meta_agent.py 128 3 60 6 95%
mesa/mesa_logging.py 82 2 14 1 95%
mesa/model.py 97 5 16 2 94%
mesa/space.py 584 40 222 17 92%
mesa/visualization/__init__.py 8 0 0 0 100%
mesa/visualization/backends/__init__.py 3 0 0 0 100%
mesa/visualization/backends/abstract_renderer.py 29 1 4 1 94%
mesa/visualization/backends/altair_backend.py 139 11 40 8 89%
mesa/visualization/backends/matplotlib_backend.py 186 20 68 9 87%
mesa/visualization/command_console.py 191 155 48 0 15%
mesa/visualization/components/__init__.py 18 6 8 1 58%
mesa/visualization/components/altair_components.py 190 82 66 11 55%
mesa/visualization/components/matplotlib_components.py 59 13 18 4 73%
mesa/visualization/components/portrayal_components.py 37 2 12 2 92%
mesa/visualization/mpl_space_drawing.py 346 83 122 31 72%
mesa/visualization/solara_viz.py 378 187 160 21 48%
mesa/visualization/space_drawers.py 291 7 74 10 95%
mesa/visualization/space_renderer.py 180 50 74 12 66%
mesa/visualization/user_param.py 23 2 4 1 89%
mesa/visualization/utils.py 4 1 0 0 75%
-------------------------------------------------------------------------------------------------
TOTAL 5815 885 1762 202 82%
============= 378 passed, 1 skipped, 1 warning in 98.00s (0:01:38) ============= with
- Change pyproject.toml to use instead of
- Add to coverage configuration
The pytest-cov plugin had conflicts with the new nested test package
structure introduced in PR mesa#2994. Using coverage.py directly with the
--cov-context flag resolves the tracing issues.
|
Nice fix @falloficarus22! Small note: original CI used |
Yeah but this is still a workaround, works until maintainers find the fix for the root cause. |
|
Thanks for the PR. What are the main advantages of this workaround over the other one? |
|
webknjaz
left a comment
There was a problem hiding this comment.
Here's a few notes from me.
| omit = ["tests/*"] | ||
|
|
||
| [tool.coverage.report] | ||
| omit = ["tests/*"] | ||
|
|
There was a problem hiding this comment.
Best not to skip collecting coverage of the test modules, nor hide it from reports.
| omit = ["tests/*"] | |
| [tool.coverage.report] | |
| omit = ["tests/*"] |
https://nedbatchelder.com/blog/202008/you_should_include_your_tests_in_coverage.html / https://nedbatchelder.com/blog/201908/dont_omit_tests_from_coverage.html / https://nedbatchelder.com/blog/201106/running_coverage_on_your_tests.html
In fact, it'd be a good idea to include an additional reported check in the Codecov config, requiring 100% code coverage on the test folder: https://github.com/ansible/awx-plugins/blob/274b32a/.codecov.yml#L33-L38.
One might argue that this is out of the scope of this PR and request a follow-up PR but can you prove that all the tests actually run in CI w/o that?
.github/workflows/build_lint.yml
Outdated
| - run: | | ||
| coverage run -m pytest tests/ | ||
| coverage xml | ||
| - run: pytest --cov=mesa --cov-report=xml --cov-context=test tests/ |
There was a problem hiding this comment.
Use --cov w/o a value to enable the plugin but not limit what's measured. It's actually a bit messy since it corresponds to source in the coveragepy config and there's no corresponding flag for source_pkgs, which means it probably refereces a folder and not an importable. Plus, as I mentioned elsewhere, test modules should really be measured too.
It's a good idea to also add -p pytest_cov which makes pytest load the plugin earlier than the standard entry point-based auto-discovery mechanism.
Finally, I'm adding --cov-report=markdown-append:"${GITHUB_STEP_SUMMARY}" since it's a nice touch to have this integrated in GHA:
| - run: pytest --cov=mesa --cov-report=xml --cov-context=test tests/ | |
| - run: pytest -p pytest_cov --cov --cov-report=xml --cov-report=markdown-append:"${GITHUB_STEP_SUMMARY}" --cov-context=test tests/ |
Follow-up PR action items:
- Move
tests/into a dedicated testpaths setting in.pytest.toml - Move
--covand--cov-context=testinto addopts in the pytest config too — these would be useful to be auto-added for both local contributors and in CI - Keep
--cov-report=xmland--cov-report=markdown-append:"${GITHUB_STEP_SUMMARY}"in CI since locally contributors don't need those reports - Configure html coverage explicitly so that the contributors could pass
--cov-report=htmllocally and have everything show up nicely by default: https://github.com/ansible/awx-plugins/blob/274b32a/.coveragerc#L1-L3 - Look into replacing
pytestwithpython -Im pytest(-Idisables auto-adding the current working directory to$PYTHONPATH/sys.pathso you'd be sure you're testing what's insite-packages/and not the Git checkout copy)
There was a problem hiding this comment.
You may want to play with hiding less important things in all reports: https://github.com/ansible/awx-plugins/blob/274b32a/.coveragerc#L17-L19. But it's best to do so combined with setting up HTML as I mentioned in another comment — this one should show everything because it's to be used interactively.
There was a problem hiding this comment.
When the copy of the library in site-packages is tested correctly, you might need to set up mappings in [paths]: https://github.com/ansible/awx-plugins/blob/274b32a/.coveragerc#L5-L12.
Note that this is only displayed in the HTML report and Codecov doesn't know how to show such contexts (it only shows own platform-specific flags you pass when uploading).
This is often considered controversial but I tend to like this way too. |
|
- Remove omit = [tests/*] to measure test coverage (as per @webknjaz) - Add source = [.] alongside source_pkgs for complete coverage - Update CI to use -p pytest_cov for early plugin loading - Add markdown coverage report to GitHub Actions summary - Use --cov without value to measure all code This ensures all tests are actually running and provides better visibility into coverage metrics.
|
@webknjaz Thanks for the detailed review and suggestions. As I have mentioned earlier this is supposed to be a temporary workaround (an improvement over #3004). I'll work on moving |
|
@falloficarus22 almost. You keep adding an empty report section which is not necessary w/o any settings in it. I usually recommend accepting suggestions via GH UI – it is likely that you'll get them committed exactly as they were meant to be, and the added bonus is that it's easier to credit people this way. Otherwise, trying to reproduce that locally is rather manual and thus error-prone: https://hynek.me/til/easier-crediting-contributors-github/. With this, you'd click commit in-browser and then pull the changes locally. Afterwards, you'd be able to make corrections/rebasing/cleanups as needed. |
|
I thought that https://github.com/mesa/mesa/pull/3006/files#r2646764846 can wait but now I realized that it must be included into this PR. Look at https://github.com/mesa/mesa/actions/runs/20505498180?pr=3006#summary-58919356331 — it lists both relative paths (with 0% for anything starting with You can take https://github.com/ansible/awx-plugins/blob/274b32a/.coveragerc#L5-L12 as is and convert it from INI into TOML, integrate that into As a side note, I noticed that some test modules don't have 100% coverage. This means that there's dead code in tests that's never exercised (but people may think that it's being run in CI, which would be misleading). This is a good candidate for figuring out in another follow-up PR — maybe, some code can be deleted, otherwise some of it might need to be marked with nocover pragma comments. |
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
webknjaz
left a comment
There was a problem hiding this comment.
This should address the last missing piece: #3006 (comment) / https://github.com/mesa/mesa/pull/3006/files#r2646764846
|
|
|
@falloficarus22 if that's the case, could you please close and then reopen the PR right away? This will trigger a new CI run. Or ask the maintainers to restart that one job. I'd like to see it post the coverage report on the CI run web page (that job Markdown summary as a table). The diff looks good but it's a good idea to double-check that the mapping works correctly before finally green-lighting the merge. |
|
Weird, but I think the PR is now good enough to be merged. |
webknjaz
left a comment
There was a problem hiding this comment.
Not weird, really. When the tests themselves are flaky, this is expected. They can usually be make more reliable or the easiest hack would be adding a pytest plugin for auto-restarting the unstable tests.
Anyway.. I see that the report looks like what I'd expect (within the context I learned): https://github.com/mesa/mesa/actions/runs/20526264823#summary-58974397795.
You're good to go now. No more notes from me.
|
Thanks for both your effort! @falloficarus22 could you update the PR description to be fully up to date with the current diff and everything we’ve learned? I would like to have it as a reference for if we ever need to revisit it. Focus on helping us understand what every option does and how they interact. |
|
@EwoutH updated the PR description. Please tell me if you would like to make any changes in this PR. |
This is partially accurate.
|
Summary
Improves coverage collection after test reorganization in #2994 by adopting
pytest's native coverage integration and measuring all code including tests.Problem
After moving tests into nested subdirectories, coverage.py reported 0% coverage. The root cause was a combination of:
Solution
This PR implements reviewer feedback to adopt coverage best practices:
Configuration Changes (
pyproject.toml):source = ["."]: Measures all code in the project directorysource_pkgs = ["mesa"]: Specifically tracks the mesa packagerelative_files = true: Fixes path resolution for nested directoriesomit = ["tests/*"]: Now measures test coverage (critical for verifying all tests run)[tool.coverage.paths]: Maps installed package paths for accurate coverage reportingCI Changes (
.github/workflows/build_lint.yml):coverage run -m pytest+coverage xml(2 commands from CI: Split off separate coverage job #3005)python -Im pytest -p pytest_cov --cov --cov-report=xml --cov-report=markdown-append:"${GITHUB_STEP_SUMMARY}" --cov-context=test tests/(single command)-Imflag ensures testing installed package, not git checkout-p pytest_covloads plugin early to avoid import issues--cov(no value) measures everything per coverage config--cov-context=testenables per-test coverage trackingAdvantages Over PR #3005
source = ["."]+source_pkgscombinationpython -ImflagTesting
Coverage by Package
Credits
Implementation incorporates excellent feedback from @webknjaz