Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add local fixtures in test_polydata #1326

Merged
merged 1 commit into from
May 18, 2021

Conversation

adeak
Copy link
Member

@adeak adeak commented May 18, 2021

The test_polydata.py module had a handful of global meshes that were being copied all over the place. This PR replaces them with module-level fixtures.

One non-trivial change is that the now-local sphere fixture (coming from pv.Sphere) shadows the global sphere fixture (coming from pv.examples.load_sphere, ultimately reading a file with data arrays). This doesn't seem like an issue and the tests run fine.

@adeak adeak added the testing Anything related to CI testing label May 18, 2021
@adeak
Copy link
Member Author

adeak commented May 18, 2021

GitHub actions are still flaky. We can wait until they come back; this one needs solid CI.

@codecov
Copy link

codecov bot commented May 18, 2021

Codecov Report

Merging #1326 (18e2b1d) into master (d6ada7b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1326   +/-   ##
=======================================
  Coverage   90.05%   90.05%           
=======================================
  Files          56       56           
  Lines       12181    12181           
=======================================
  Hits        10969    10969           
  Misses       1212     1212           

Copy link
Member

@akaszynski akaszynski left a comment

Choose a reason for hiding this comment

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

I restarted CI and it seems that it ran fine. Test run locally with pytest -vx -n 4 on Linux.

I see no issue using a different sphere fixture other than we might consider replacing the global fixture in a later PR with one like this (if we're fine giving up the data_arrays).

@akaszynski
Copy link
Member

You've got merge permission and can fire when ready.

@adeak adeak merged commit 587f3ef into pyvista:master May 18, 2021
@adeak
Copy link
Member Author

adeak commented May 18, 2021

Thanks!

@adeak adeak deleted the fix/sphere_fixtures_in_polydata branch May 18, 2021 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Anything related to CI testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants