-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add screenshot in pyvista related methods #521
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
Conversation
i will integrate it in my PR |
@StefanThoene Reopening for testing purposes |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd use the test name for the image name :)
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Description
Adding
screenshot
option topreview
methods. On top of it, the CI and test configuration has been updated to allow this kind of test to without issues. This will allow us to increase the code coverage !!!Issue linked
None
Checklist
feat: add optical property
)Previous PR content, kept for visibility
To increase code coverage (and fix bugs, see #520) this PR starts to add test(s) on a method using pyvista rendering.Three options are available to do so:leverage an external github action to use headless display;use the python package(https://docs.vtk.org/en/latest/advanced/available_python_wheels.html);vtk-osmesa
from vtk, see [available python wheels]patch the call toshow
so that we don't really trigger them.While I like the two first options as they would let us really test everything, they also come with a security risk as we don't maintain the solution associated (tbh, I see little risk in using them though). Therefore, I'm following the third option. Depending on what you think about this, we could make this PR evolve.