Adding plot testing

[germa89]

As the title.

Let's use the pytest plugin pytest-pyvista to compare plots and detect changes. This is going to add a bit of burden to the developer since we might need to update the image cache every time there are changes in anything related to plotting.

But overall, I think this is a very good improvement, because there is the check and also there is the cache. We can inspect manually the cache and see if there is any plot suspicious.

Notes

I did have to disable the cache of some tests because they seemed to change quite a lot between run or between the machine I ran the test locally and the CICD.

I wonder if there is a way to make the CICD update those images. Probably, part of the nightly/scheduled build should be open an issue when important differences are found.

Skipped tests:

Look for:

verify_image_cache.skip = True

The final CICD run showed the following fails:

[FAILED] test_disp_plot_subselection - E           RuntimeError: test_disp_plot_subselection Exceeded image regression error of 500.0 with an image error equal to: 1149.7163398692887
[FAILED] test_nodal_eqv_stress - E        +    where <function allclose at 0x7fc65def0eb0> = np.allclose
[FAILED] test_plot_nodal_eqv_stress - E           RuntimeError: test_plot_nodal_eqv_stress Exceeded image regression error of 500.0 with an image error equal to: 3898.7856209142265
[FAILED] test_general_plotter_returns - E           RuntimeError: test_general_plotter_returns_1 Exceeded image regression error of 500.0 with an image error equal to: 86640.32287580681
= 4 failed, 1253 passed, 231 skipped, 2 xfailed, 12 warnings, 33 rerun in 872.82s (0:14:32) =

Increased variance

Only one test at the moment

def test_plot_element_values(mapdl, static_solve, verify_image_cache):
    verify_image_cache.high_variance_test=600

[codecov]

Codecov Report

Merging #2299 (032be0e) into main (e3885fb) will decrease coverage by 4.64%.
Report is 121 commits behind head on main.
The diff coverage is 87.45%.

@@            Coverage Diff             @@
##             main    #2299      +/-   ##
==========================================
- Coverage   86.39%   81.76%   -4.64%     
==========================================
  Files          45       45              
  Lines        7977     8435     +458     
==========================================
+ Hits         6892     6897       +5     
- Misses       1085     1538     +453     

[AlejandroFernandezLuces]

Overall, the usage of the plugin looks good to me. I have a comment about the image_cache and the different approaches to save it.

The approach you are using, saving directly the images in the repository, is the one used in pyvista. This works well, but can bloat the repository quickly as the amount of plotting tests increase.

In PyAnsys, the approach we are generally using is to upload the image_cache as an artifact. In each test run, we download the images if available, run the tests and then reupload the images. If you need to reset the cache, you can update the "reset version" of the artifact by changing the reset number and it will generate a new version.

You can see some examples of this in PyGeometry and PyPrimeMesh:

https://github.com/ansys/pyprimemesh/blob/1aee402e03195f1e4aa6f859055b1519454b637a/.github/workflows/ci_cd.yml#L125C1-L147C28
https://github.com/ansys/pyansys-geometry/blob/2a543290e8dbacc6d2d5f25a62859b5e5976bb9b/.github/workflows/ci_cd.yml#L132-L158

If this implementation is doable in PyMAPDL, I encourage you to do so, feel free to ask me if you have any doubts 😄

[germa89]

@AlejandroFernandezLuces

Thank you a lot for your detailed comment. I think the idea is good. It will decluter the repo a bit.

However, to better have a grasp of the plot differences between local and CICD, I will keep the cache in the repo for now at least for now since we are not very familiar with this image cache checking.

Later in the future, I will implement the change you mention once I'm confident on those changes.
I will also like to get rid of the test exceptions I had to implement. But I didn't want to spend too much time on this.

Again. I will follow your advice in a near future. Thank you a lot!

[germa89]

LGTM ... because it LGT @AlejandroFernandezLuces

[pyansys-ci-bot]

✅ Approving this PR because germa89 said so in here 😬