Skip to content

test: improving tests performance#4425

Open
germa89 wants to merge 15 commits intomainfrom
tests/improving-tests-performance
Open

test: improving tests performance#4425
germa89 wants to merge 15 commits intomainfrom
tests/improving-tests-performance

Conversation

@germa89
Copy link
Collaborator

@germa89 germa89 commented Feb 18, 2026

Description

Some changes to improve testing:

  • Not checking for extra MAPDL instances unless we are on DEBUG_TESTING
  • Do not recreate the glyphs on each call.

Issue linked

Close #3568

Checklist

@germa89 germa89 self-assigned this Feb 18, 2026
@germa89 germa89 requested a review from a team as a code owner February 18, 2026 12:32
@germa89 germa89 changed the title tests: improving tests performance test: improving tests performance Feb 18, 2026
@github-actions github-actions bot added dependencies maintenance General maintenance of the repo (libraries, cicd, etc) labels Feb 18, 2026
@github-actions github-actions bot added enhancement Improve any current implemented feature and removed dependencies maintenance General maintenance of the repo (libraries, cicd, etc) labels Feb 18, 2026
@germa89
Copy link
Collaborator Author

germa89 commented Feb 18, 2026

Testing if the image cache gets changed.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR targets faster (and less intrusive) test runs and plotting initialization by reducing per-test cleanup work and avoiding repeated glyph construction, and it also relaxes the optional dependency constraint for the visualization interface.

Changes:

  • In test teardown, only scan/stop “extra” MAPDL instances when DEBUG_TESTING is enabled.
  • Change plotting defaults so boundary-condition glyphs are configured once (cached) instead of being recreated on every access.
  • Relax the ansys-tools-visualization-interface upper bound in optional dependencies.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
tests/conftest.py Gates expensive “extra MAPDL instance” cleanup behind DEBUG_TESTING.
src/ansys/mapdl/core/plotting/plotting_defaults.py Switches BC glyph defaults to one-time configuration/caching.
pyproject.toml Expands allowed versions of ansys-tools-visualization-interface for optional installs.
Comments suppressed due to low confidence (1)

src/ansys/mapdl/core/plotting/plotting_defaults.py:78

  • Switching back to one-time configuration means DefaultSymbol.__call__ returns the same mutable dict/PolyData instances on every access. Downstream plotting (notably legend creation and any in-place PyVista transforms) can mutate these meshes/dicts, which matches the glyph offset/flakiness described in #3568 and can reintroduce it. Consider returning defensive copies (e.g., copy the dict and deep-copy the glyph mesh) or storing factories/immutable templates so each call gets a fresh glyph object while still avoiding recomputing parameters unnecessarily.
        if not self._configured:  # Temporal patch pending on #3568
            self._set_configuration()
            self._configured = True

        return getattr(self, name)

Comment on lines +74 to 75
if not self._configured: # Temporal patch pending on #3568
self._set_configuration()
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

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

The inline note says this is a “Temporal patch pending on #3568”, but this PR is marked to close #3568. If the issue is now resolved, this comment should be updated/removed; if it’s not resolved, the PR description/linking should be adjusted because this change reverts the prior mitigation.

Copilot uses AI. Check for mistakes.
Comment on lines +74 to 76
if not self._configured: # Temporal patch pending on #3568
self._set_configuration()
self._configured = True
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

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

There’s no regression coverage around the behavior that prompted #3568 (glyphs/legend geometry drifting across calls/tests). Since this change alters caching semantics, adding a test that exercises repeated BC plotting/legend creation and asserts glyph geometry stays stable would help prevent reintroducing the flakiness.

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Feb 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.06%. Comparing base (b2a2ed3) to head (57aff99).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4425      +/-   ##
==========================================
- Coverage   91.16%   91.06%   -0.11%     
==========================================
  Files         196      196              
  Lines       16209    16209              
==========================================
- Hits        14777    14760      -17     
- Misses       1432     1449      +17     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improve any current implemented feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plot changes when DEFAULTS are not rebuild.

3 participants

Comments