Bump pytest-pyvista and fix image regression testing

[banesullivan]

Adds the cache dir to the pytest ini options for pyvista/pytest-pyvista#19

cc @MatthewFlamm

[codecov]

Codecov Report

Merging #3748 (75c3bdf) into main (ff6939a) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3748      +/-   ##
==========================================
+ Coverage   93.99%   94.00%   +0.01%     
==========================================
  Files          82       82              
  Lines       18553    18553              
==========================================
+ Hits        17439    17441       +2     
+ Misses       1114     1112       -2     

[MatthewFlamm]

LGTM. I recommend waiting to merge this until a release of pytest-pyvista after pyvista/pytest-pyvista#19 is merged. We might find that we have failing tests once that happens since the image regression tests are currently broken.

[banesullivan]

We might find that we have failing tests once that happens since the image regression tests are currently broken.

They should be working on CI actually as the --image_cache_dir option is set in the invocation of the pytest command in each of the workflow yaml files

[MatthewFlamm]

They should be working on CI actually as the --image_cache_dir option is set in the invocation of the pytest command in each of the workflow yaml files

Ah makes sense. Nevermind then. There may still be image regression issues, but not due to this.

[MatthewFlamm]

Raising an error for vtk8 was a costly one. There is some value to running the tests but not do the image cache test, but it is also dangerous for other users of pytest-pyvista who will assume the images are being compared. I can think of two paths forward. We could either revert that change in pytest-pyvista or skip testing this entire module (referring to tests/plotting/test_plotting). A third option would be to only skip tests that do pyvista.show or similar, but this would be more costly to implement.

Once these packages have settled together it would be great to run these tests inside pytest-pyvista to ensure we don't encounter these issues all the time.

[MatthewFlamm]

I think we should allow the option to skip tests, but default to raising an error. To me, this prevents unexpected behavior while providing easy flexibility opt-in.

[MatthewFlamm]

We need to get this merged soon as it is starting to hold up other PRs. See #3761.

We need to merge pyvista/pytest-pyvista#26 and make a new release, unless there are other suggestions on how to fix. There are likely other image regressions that need to be fixed subsequently.

[MatthewFlamm]

They should be working on CI actually as the --image_cache_dir option is set in the invocation of the pytest command in each of the workflow yaml files

Ah makes sense. Nevermind then. There may still be image regression issues, but not due to this.

This flag was pointing to ./image_cache, which was incorrect. I'm not sure how this didn't cause failures.

[banesullivan]

Well, at least now we are actually getting image regression failures!

[banesullivan]

I think this should get merged as a hotfix ASAP then we should implement pyvista/pytest-pyvista#22 and audit any image cache skipping or high variance settings in a follow up PR

[MatthewFlamm]

#3579 modified the test_ssao_pass more than just removing the callback. We probably need to revert those changes?

[MatthewFlamm]

I think we've gotten to a point where there are only a few tests are just failing the regression check on mostly Windows. It's a bit weird since the tests I checked haven't significantly changed recently. But it may be worth applying a skip windows or a high variance to those at this point. We might want to add "high_variance_windows" attributes in the future.

[MatthewFlamm]

It passes!

[banesullivan]

Alright! Let's merge