-
Notifications
You must be signed in to change notification settings - Fork 18
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
Use tmpdir for povray rendering #675
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #675 +/- ##
==========================================
+ Coverage 83.33% 83.37% +0.03%
==========================================
Files 17 17
Lines 3583 3591 +8
==========================================
+ Hits 2986 2994 +8
Misses 597 597
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -107,6 +109,10 @@ def test_structure_data_viewer_storage(structure_data_object): | |||
# fmt: on | |||
v._render_structure() | |||
|
|||
# Make sure we don't polute current working dir with tempfiles |
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.
Thanks for the fix, but is this necessary for the test to pass? If it pollutes the working dir, will the test fail?
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.
Yes, it's a regression test for this PR.
03525ce
to
d06764a
Compare
@superstar54 I've rebased this branch to get rid of the unrelated changes, please take a look. |
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.
LGTM!
Stacked on top of #627 which needs to be merged first.Fixes #676