tests: consolidate Solara viz tests and restore Altair coverage #2993 by DipayanDasgupta · Pull Request #3011 · mesa/mesa

DipayanDasgupta · 2025-12-24T11:09:02Z

Summary

This PR refactors the Solara visualization tests into a modern/legacy structure, restores missing test coverage for the Altair backend, and improves the overall robustness of the test suite based on reviewer feedback. It removes redundant files and ensures both Matplotlib and Altair backends have feature parity in their testing.

Bug / Issue

Fixes #2993

Problem

A recent test reorganization inadvertently removed test coverage for Altair's AgentPortrayalStyle support.
Duplicate and outdated test files (test_solara_viz_updated.py) created ambiguity and maintenance overhead.
The legacy and modern testing APIs were mixed, making the test suite harder to navigate and maintain.

Implementation

Structural Alignment: Migrated all visualization tests to the new tests/visualization/ directory, aligning with the project's goal to mirror the source module structure (#2994).

Backend Parity: Implemented @pytest.mark.parametrize in test_solara_viz.py to test both Matplotlib and Altair backends against the same modern SpaceRenderer logic, ensuring 100% parity and preventing future coverage gaps.

Restored Coverage & Robustness: Based on reviewer feedback, the tests were significantly improved:
Assertions are now more robust, using spy_*.assert_called_with(renderer) to verify that the correct objects are being passed to draw functions.
Added specific isinstance checks to confirm that the correct backend renderer class (MatplotlibBackend or AltairBackend) is instantiated.
Added negative test cases to confirm nothing is drawn when a renderer is not provided.

Legacy Preservation: Moved all tests for deprecated, dictionary-based portrayals to test_solara_viz_legacy.py. This now includes dedicated tests for both agent and property layer portrayals, ensuring full backward compatibility.
Refactoring: Removed the redundant test_solara_viz_updated.py file and the unnecessary test_voronoi_grid_renderer function, consolidating all logic into the two primary test files.

Testing

The test suite is now cleanly organized into two files, with all tests passing locally and in CI.

tests/visualization/test_solara_viz.py: Contains 11 tests validating the modern API and SpaceRenderer for both backends. tests/visualization/test_solara_viz_legacy.py: Contains 3 tests validating the legacy, dictionary-based APIs to ensure backward compatibility.

Additional Notes

This PR strictly follows the two-file structure requested by the maintainers: one primary file for modern tests and one legacy file for deprecated features.
The high deletion count in the diff is due to the removal of the redundant intermediate file (test_solara_viz_updated.py) and the consolidation of logic.

github-actions · 2025-12-24T11:16:17Z

Performance benchmarks:

Model	Size	Init time [95% CI]	Run time [95% CI]
BoltzmannWealth	small	🔵 -0.7% [-1.1%, -0.4%]	🔵 -0.6% [-0.8%, -0.5%]
BoltzmannWealth	large	🔵 +1.7% [-1.1%, +7.0%]	🔵 -3.7% [-5.3%, -2.2%]
Schelling	small	🔵 -0.9% [-2.5%, +0.9%]	🔵 -1.5% [-2.5%, -0.6%]
Schelling	large	🔵 +0.8% [-0.6%, +3.4%]	🔵 -1.3% [-2.3%, -0.2%]
WolfSheep	small	🔵 -1.1% [-2.2%, -0.2%]	🔵 -0.3% [-0.5%, -0.1%]
WolfSheep	large	🔵 -3.2% [-8.3%, -0.3%]	🔵 -0.8% [-1.7%, +0.0%]
BoidFlockers	small	🔵 -0.1% [-0.6%, +0.3%]	🔵 -0.4% [-0.6%, -0.3%]
BoidFlockers	large	🔵 +0.1% [-0.3%, +0.6%]	🔵 -0.4% [-0.6%, -0.3%]

DipayanDasgupta · 2025-12-24T11:21:22Z

@coderabbitai review

coderabbitai · 2025-12-24T11:21:37Z

Tip

For best results, initiate chat on the files or code changes.

@DipayanDasgupta: Hello! How can I assist you with this pull request? Would you like me to:

Review the changes in this PR?
Help with a specific file or code section?
Answer questions about the test consolidation or Altair coverage?
Generate additional tests or documentation?
Something else?

Please let me know what you need!

coderabbitai · 2025-12-24T11:21:39Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2025-12-24T11:24:23Z

Walkthrough

The pull request consolidates Solara visualization test coverage by adding modern Altair backend support testing to test_solara_viz.py and creating a new test_solara_viz_legacy.py file to verify backwards compatibility with legacy dictionary-based portrayals.

Changes

Cohort / File(s)	Summary
Modern Altair Support `tests/visualization/test_solara_viz.py`	Adds `test_modern_renderer_altair_support()` to test `SpaceRenderer` with Altair backend, including portrayal function validation and renderer backend assertion. Imports `SpaceRenderer` for new test.
Legacy Backwards Compatibility `tests/visualization/test_solara_viz_legacy.py`	New test file verifying dict-based portrayal functions work with legacy component APIs (`make_mpl_space_component`, `make_altair_space`). Tests `MockModel` rendering via `SolaraViz` with both Matplotlib and Altair configurations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Poem

🐰 A tale of tests both old and new,
We organize our rabbit's brew—
Modern paths and legacy ways,
Backwards-compatible through all our days!
Altair sparkles, portrayals align,
Test coverage makes our code divine! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main changes: consolidating Solara visualization tests and restoring Altair coverage, with reference to the issue number.
Linked Issues check	✅ Passed	The PR successfully addresses all key objectives from issue #2993: creates a primary test file with modern APIs and Altair coverage, isolates legacy dict-based tests, removes redundant files, and consolidates logic as required.
Out of Scope Changes check	✅ Passed	All changes are directly related to the stated objectives: adding new tests, organizing test files, and removing redundancy. No unrelated modifications are present.
Description check	✅ Passed	The PR description is comprehensive and well-structured, clearly detailing the problem, implementation, testing, and additional notes that address the repository's templates and expectations.

✨ Finishing touches

📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (2)

tests/visualization/test_solara_viz_legacy.py (1)
15-20: Consider adding agents to validate portrayal rendering.

The MockModel creates a grid but doesn't place any agents. This means the agent_portrayal function won't actually be exercised during rendering, making the test less thorough in validating legacy dictionary-based portrayal behavior.
🔎 Suggested enhancement to add agent validation
 class MockModel(mesa.Model):
     def __init__(self):
         super().__init__()
         self.grid = MultiGrid(10, 10, True)
+        # Create and place an agent to exercise the portrayal function
+        agent = mesa.Agent(self)
+        self.grid.place_agent(agent, (5, 5))
tests/visualization/test_solara_viz.py (1)
217-235: Test successfully addresses Altair coverage gap; consider enhanced validation.

The test correctly demonstrates SpaceRenderer with Altair backend support using modern APIs (AgentPortrayalStyle). It fulfills the PR objective of restoring Altair coverage identified in issue #2993.
💡 Optional: Enhance test validation

Consider adding verification that the renderer actually produced output and that the portrayal function was exercised:
def test_modern_renderer_altair_support():
    """Verify SpaceRenderer with Altair and AgentPortrayalStyle (Fixes #2993 coverage)."""

    class MockModel(mesa.Model):
        def __init__(self):
            super().__init__()
            self.grid = MultiGrid(10, 10, True)
            self.grid.place_agent(mesa.Agent(self), (5, 5))

    model = MockModel()
    
    portrayal_called = False
    def portrayal(agent):
        nonlocal portrayal_called
        portrayal_called = True
        return AgentPortrayalStyle(marker="o", color="red")

    # This tests the new architecture with the Altair backend
    renderer = SpaceRenderer(model, backend="altair").setup_agents(portrayal).render()
    solara.render(SolaraViz(model, renderer))
    
    assert renderer.backend == "altair"
    assert portrayal_called, "Portrayal function should have been called during rendering"
    assert renderer.agent_mesh is not None, "Agent mesh should be created after render"
This validates that the rendering pipeline actually executed the portrayal logic.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b5eead and 8503a9b.

📒 Files selected for processing (2)

tests/visualization/test_solara_viz.py
tests/visualization/test_solara_viz_legacy.py

🧰 Additional context used

🧬 Code graph analysis (1)

tests/visualization/test_solara_viz_legacy.py (6)

mesa/space.py (1)

MultiGrid (1013-1086)

mesa/visualization/solara_viz.py (1)

SolaraViz (58-221)

mesa/visualization/components/altair_components.py (1)

make_altair_space (27-61)

mesa/visualization/components/matplotlib_components.py (1)

make_mpl_space_component (25-60)

mesa/model.py (1)

Model (30-255)

mesa/visualization/space_renderer.py (1)

render (381-409)

🪛 Ruff (0.14.10)

tests/visualization/test_solara_viz_legacy.py

22-22: Unused function argument: agent

(ARG001)

tests/visualization/test_solara_viz.py

228-228: Unused function argument: agent

(ARG001)

🔇 Additional comments (2)

tests/visualization/test_solara_viz_legacy.py (1)

1-11: LGTM! Imports are appropriate for legacy visualization testing.

The imports correctly include the legacy component makers (make_mpl_space_component and make_altair_space) alongside the core visualization infrastructure.

tests/visualization/test_solara_viz.py (1)

25-25: LGTM! Import correctly supports the new Altair backend test.

The SpaceRenderer import is appropriately added to enable testing of the modern renderer API with Altair support, addressing the coverage gap identified in issue #2993.

tests/visualization/test_solara_viz_legacy.py

DipayanDasgupta · 2025-12-24T18:51:34Z

@ShreyasN707 @Nithin9585 Good catch! I have just pushed an update that explicitly removes test_solara_viz_updated.py.
The visualization test suite is now strictly consolidated into two files:
test_solara_viz.py: The single source of truth for modern API tests (including the restored Altair coverage).
test_solara_viz_legacy.py: Reserved for deprecated dictionary-based portrayal behavior.
This should remove any remaining ambiguity. Thanks for the review!

DipayanDasgupta · 2025-12-24T19:12:17Z

I have updated PR #3011. It consolidates the tests into tests/visualization/ (aligned with #2994) but surpasses other solutions by using Pytest Parameterization to ensure 100% parity between Matplotlib and Altair. This ensures that any logic verified for one backend is automatically verified for the other, preventing future coverage gaps.

…ameterization mesa#2993

EwoutH · 2025-12-27T16:11:59Z

Thanks for this effort.

@Nithin9585 @codebyNJ @ShreyasN707 please review using the "review" tab on this PR.

@DipayanDasgupta can you update the PR description and format it properly?

ShreyasN707

@DipayanDasgupta The consolidation looks good and the modern vs legacy split is nice and clear now.

I only noticed a couple of small, non-blocking nits:

There are a few leftover # CodeRabbit Rec: comments in test_solara_viz_legacy.py that look like AI artifacts and could be cleaned up.
(Optional) In the backend parity test, using spy.call_count >= 1 would be a slightly more robust check than spy.called, though the current assertion is fine.

DipayanDasgupta · 2025-12-27T18:11:47Z

@EwoutH I have updated the PR description with proper formatting and structure.

@ShreyasN707 Thanks for the review! I have pushed a fix to remove the leftover AI comments from the legacy file. Regarding the assertion, I stuck with assert spy.called for now since the strict call count check was optional, but I'm happy to add it if you think it's necessary for merging.

DipayanDasgupta · 2025-12-29T04:39:42Z

@EwoutH could you give the final review , and let me know if the PR is ready for merging?

EwoutH

I think this looks good, thanks!

@Sahil-Chhoker could you double check, especially if everything is covered now?

Sahil-Chhoker · 2025-12-29T08:30:57Z

This PR is important for visualization test suite, I'll do a full review later today but before that I want to know that if test_solara_viz.py contains tests for newer API why does it have these tests for older API:

Also if these are all the tests for the new API why is propertylayer drawing function not tested here?

I took a look in a hurry, just point me out if I'm wrong.

DipayanDasgupta · 2025-12-29T10:04:53Z

@Sahil-Chhoker Thanks for the detailed review! I've updated the PR to address both points:

Removed Old API Tests: I removed test_call_space_drawer completely as you suggested, since it was testing the legacy API which doesn't belong in the modern test file.
Added PropertyLayer Coverage: I updated test_solara_viz_backends to include a PropertyLayer in the mock model and explicitly assert that draw_propertylayer is called (spy_properties.call_count >= 1).

This ensures the modern test file is focused purely on the new architecture and covers the property layer rendering logic.

EwoutH · 2025-12-29T13:02:12Z

I removed test_call_space_drawer completely as you suggested, since it was testing the legacy API which doesn't belong in the modern test file.

Are these tests still relevant? If so, shouldn't it be moved to the legacy one?

@Sahil-Chhoker

- Remove test_call_space_drawer from test_solara_viz.py (modern API only) - Move test_call_space_drawer to test_solara_viz_legacy.py - Preserves coverage of make_mpl_space_component/make_altair_space APIs - Modern file: 12 tests (SpaceRenderer only) - Legacy file: 2 tests (backwards compatibility) Addresses feedback from @Sahil-Chhoker and @EwoutH in mesa#3011

DipayanDasgupta · 2025-12-29T14:25:31Z

@EwoutH You're right they are relevent, I was in a bit of a rush , so I kind of overlooked that. I've restored the test_call_space_drawer logic and moved it into test_solara_viz_legacy.py. This ensures we keep coverage for make_mpl_space_component and other legacy APIs without cluttering the modern test file. All 14 tests are passing locally.

@Sahil-Chhoker

- Remove test_call_space_drawer (tests old make_mpl_space_component API) - Add VoronoiGrid import to fix test_voronoi_grid_renderer - Keep PropertyLayer coverage in test_solara_viz_backends per review - All 12 tests passing with modern SpaceRenderer API only Addresses feedback from @Sahil-Chhoker in mesa#3011

Sahil-Chhoker · 2025-12-29T15:55:58Z

@DipayanDasgupta Thanks for incorporating the review and sorry I won't be able to find time today but will look at it as soon as tomorrow.

Sahil-Chhoker

The test suite is missing the portion of original test that was used to verify the correct objects were being passed to the called functions.
Considering one example:

there may be more, please add these as well.

tests/visualization/test_solara_viz.py

tests/visualization/test_solara_viz_legacy.py

DipayanDasgupta · 2025-12-31T11:54:08Z

The test suite is missing the portion of original test that was used to verify the correct objects were being passed to the called functions. Considering one example:

Thanks for pointing this out! I've restored the more robust assertions. The test now uses spy_*.assert_called_with(renderer) to verify the correct objects are being passed. I also added back the check to ensure nothing is drawn when no renderer is passed.

DipayanDasgupta · 2025-12-31T11:56:42Z

Hi @Sahil-Chhoker, thank you for the detailed and helpful review! I've pushed an update that incorporates all of your feedback.

The changes include:

Restored the robust assert_called_with(renderer) assertions.
Added specific backend isinstance checks.
Added a new dedicated test for legacy property layer portrayals.
Removed the redundant Voronoi grid test.

The tests are passing locally. Let me know if there is anything else

Sahil-Chhoker

Good work, just a little cleanup.

tests/visualization/test_solara_viz.py

tests/visualization/test_solara_viz_legacy.py

tests/examples/test_examples_viz.py

…payanDasgupta/mesa into consolidate-solara-tests-2993

Sahil-Chhoker

Good Work, LGTM, @EwoutH do you have anything to add before merging this PR?

DipayanDasgupta · 2026-01-02T09:20:22Z

@EwoutH @Sahil-Chhoker Any Updates?

EwoutH · 2026-01-02T09:25:36Z

I’m good, @DipayanDasgupta thanks for the PR and @Sahil-Chhoker thanks for the extensive review!

DipayanDasgupta mentioned this pull request Dec 24, 2025

Consolidate Solara Visualization test coverage #2993

Closed

coderabbitai bot reviewed Dec 24, 2025

View reviewed changes

tests/visualization/test_solara_viz_legacy.py Outdated Show resolved Hide resolved

DipayanDasgupta force-pushed the consolidate-solara-tests-2993 branch 2 times, most recently from 50e61b3 to 963f177 Compare December 24, 2025 18:50

DipayanDasgupta force-pushed the consolidate-solara-tests-2993 branch 2 times, most recently from 2dc9a6a to 0debdd6 Compare December 24, 2025 19:11

DipayanDasgupta force-pushed the consolidate-solara-tests-2993 branch 3 times, most recently from 24d4784 to ad94d3d Compare December 25, 2025 09:42

DipayanDasgupta added 2 commits December 25, 2025 15:17

tests: consolidate Solara viz tests and ensure backend parity via par…

6a4f15b

…ameterization mesa#2993

tests: fix call count assertions in solara viz tests mesa#2993

2667f05

DipayanDasgupta force-pushed the consolidate-solara-tests-2993 branch from ad94d3d to 2667f05 Compare December 25, 2025 09:47

Nithin9585 mentioned this pull request Dec 27, 2025

tests: consolidate Solara visualization test files #2995

Closed

ShreyasN707 approved these changes Dec 27, 2025

View reviewed changes

EwoutH approved these changes Dec 29, 2025

View reviewed changes

EwoutH requested a review from Sahil-Chhoker December 29, 2025 08:12

EwoutH added the testing Release notes label label Dec 29, 2025

DipayanDasgupta force-pushed the consolidate-solara-tests-2993 branch from b730d06 to 046aa34 Compare December 29, 2025 09:59

tests: address review nits (cleanup comments, improve assertions)

1110bc2

DipayanDasgupta force-pushed the consolidate-solara-tests-2993 branch from 046aa34 to 1110bc2 Compare December 29, 2025 10:12

Sahil-Chhoker requested changes Dec 30, 2025

View reviewed changes

tests/visualization/test_solara_viz.py Outdated Show resolved Hide resolved

tests/visualization/test_solara_viz.py Outdated Show resolved Hide resolved

Sahil-Chhoker reviewed Dec 30, 2025

View reviewed changes

tests/visualization/test_solara_viz_legacy.py Show resolved Hide resolved

Sahil-Chhoker reviewed Dec 30, 2025

View reviewed changes

tests/visualization/test_solara_viz_legacy.py Show resolved Hide resolved

Sahil-Chhoker and others added 4 commits December 30, 2025 20:59

Merge branch 'main' into consolidate-solara-tests-2993

dc0a3c6

Merge branch 'main' into consolidate-solara-tests-2993

14e0cd8

Merge branch 'main' into consolidate-solara-tests-2993

09ad467

fix: Correct assertions and model setup in legacy tests

dcb4875

DipayanDasgupta force-pushed the consolidate-solara-tests-2993 branch from 04a44cf to 2bd06d3 Compare December 31, 2025 11:49

Sahil-Chhoker reviewed Dec 31, 2025

View reviewed changes

Merge branch 'consolidate-solara-tests-2993' of https://github.com/Di…

9e429de

…payanDasgupta/mesa into consolidate-solara-tests-2993

DipayanDasgupta force-pushed the consolidate-solara-tests-2993 branch from 2bd06d3 to 9e429de Compare December 31, 2025 12:41

remove ubuntu fail fix

aa84b84

Sahil-Chhoker approved these changes Dec 31, 2025

View reviewed changes

EwoutH merged commit 8419b19 into mesa:main Jan 2, 2026
14 checks passed

Uh oh!

Conversation

DipayanDasgupta commented Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Bug / Issue

Problem

Implementation

Testing

Additional Notes

Uh oh!

github-actions bot commented Dec 24, 2025

Uh oh!

DipayanDasgupta commented Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Dec 24, 2025

Uh oh!

coderabbitai bot commented Dec 24, 2025

Uh oh!

coderabbitai bot commented Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DipayanDasgupta commented Dec 24, 2025

Uh oh!

DipayanDasgupta commented Dec 24, 2025

Uh oh!

EwoutH commented Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShreyasN707 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

I only noticed a couple of small, non-blocking nits:

Uh oh!

DipayanDasgupta commented Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DipayanDasgupta commented Dec 29, 2025

Uh oh!

EwoutH left a comment

Choose a reason for hiding this comment

Uh oh!

Sahil-Chhoker commented Dec 29, 2025

Uh oh!

DipayanDasgupta commented Dec 29, 2025

Uh oh!

EwoutH commented Dec 29, 2025

Uh oh!

DipayanDasgupta commented Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sahil-Chhoker commented Dec 29, 2025

Uh oh!

Sahil-Chhoker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DipayanDasgupta commented Dec 31, 2025

Uh oh!

DipayanDasgupta commented Dec 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sahil-Chhoker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DipayanDasgupta commented Dec 24, 2025 •

edited

Loading

DipayanDasgupta commented Dec 24, 2025 •

edited

Loading

coderabbitai bot commented Dec 24, 2025 •

edited

Loading

EwoutH commented Dec 27, 2025 •

edited

Loading

ShreyasN707 left a comment •

edited

Loading

DipayanDasgupta commented Dec 27, 2025 •

edited

Loading

DipayanDasgupta commented Dec 29, 2025 •

edited

Loading

DipayanDasgupta commented Dec 31, 2025 •

edited

Loading