Skip to content

test: refactor shared 2D mesh helper into pytest fixture#5342

Merged
agriyakhetarpal merged 4 commits intopybamm-team:mainfrom
swastim01:feat/test-mesh-fixtures
Feb 4, 2026
Merged

test: refactor shared 2D mesh helper into pytest fixture#5342
agriyakhetarpal merged 4 commits intopybamm-team:mainfrom
swastim01:feat/test-mesh-fixtures

Conversation

@swastim01
Copy link
Contributor

@swastim01 swastim01 commented Jan 10, 2026

Description

This PR refactors the reusable 2D mesh generation helper from tests/shared.py into a pytest fixture defined in tests/conftest.py.
The change removes repeated mesh construction boilerplate across multiple test files and aligns with recommended fixture usage in PyBaMM tests.

Several tests have been updated to consume the shared fixture directly, while preserving existing test semantics.

Context: This follows the discussion and suggestion in #5294 and #5295 regarding using fixtures for shared setup with substantial reuse.

Contributes to issue: #4502

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #)

Important checks:

Please confirm the following before marking the PR as ready for review:

  • No style issues: nox -s pre-commit
  • All tests pass: nox -s tests
  • The documentation builds: nox -s doctests
  • Code is commented for hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

@swastim01 swastim01 requested a review from a team as a code owner January 10, 2026 05:31
Copy link
Member

@agriyakhetarpal agriyakhetarpal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, thanks, @swastim01! I've launched the workflows and we will be good to go with this once the tests pass. One more change: since get_mesh_for_testing_2d is no longer used, could you please remove it?

Copy link
Member

@agriyakhetarpal agriyakhetarpal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@swastim01 almost there, just one more import to remove – see failing tests!

@swastim01
Copy link
Contributor Author

@swastim01 almost there, just one more import to remove – see failing tests!

Hey @agriyakhetarpal, I missed that earlier, thanks for catching it! I’ve removed the leftover import now.

Copy link
Member

@agriyakhetarpal agriyakhetarpal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good now, sorry this took me so long to review @swastim01! Thank you! I'll merge when tests pass

@codecov
Copy link

codecov bot commented Feb 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.03%. Comparing base (9307a75) to head (92b2a18).
⚠️ Report is 29 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5342      +/-   ##
==========================================
- Coverage   98.60%   98.03%   -0.57%     
==========================================
  Files         324      327       +3     
  Lines       28219    28690     +471     
==========================================
+ Hits        27824    28126     +302     
- Misses        395      564     +169     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@agriyakhetarpal agriyakhetarpal merged commit c18ea21 into pybamm-team:main Feb 4, 2026
23 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants