-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Use temp directory for os test #849
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
Codecov Report
@@ Coverage Diff @@
## master #849 +/- ##
==========================================
+ Coverage 62.89% 62.99% +0.09%
==========================================
Files 88 88
Lines 14547 14547
Branches 3288 3288
==========================================
+ Hits 9150 9164 +14
+ Misses 3238 3222 -16
- Partials 2159 2161 +2
Continue to review full report at Codecov.
|
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, I'll merge once CI is passing. Also, :) חג שמח
tests/snippets/stdlib_os.py
Outdated
base_folder = os.environ["TEMP"] | ||
else: | ||
base_folder = "/tmp" | ||
name = base_folder + os.sep + "test_os" |
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.
Could we prefix "test_os"
so that if the test failed and someone's cleaning out their temp folder, they could know what it was for? Maybe "rustpython_test_os"
?
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.
Done
tests/snippets/stdlib_os.py
Outdated
for f in os.listdir(self.name): | ||
# Currently don't support dir delete. | ||
os.remove(self.name + os.sep + f) | ||
os.rmdir(self.name) |
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.
It looks like this is what's causing CI to fail, I think this SO answer has a good solution for it with what modules we have right now.
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 changed it to not delete the folder. This fails only on rustpython and not in Cpython so I will try to solve this at a later stage. The test folder is in temp and very small so that should not be a problem.
Great. חג שמח 😃 |
This should allow us to extend filesystem tests.