-
Notifications
You must be signed in to change notification settings - Fork 206
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
TestCase : Temp directory access improvements #6204
Conversation
027810e
to
2df5d22
Compare
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 Eric! Could you address my minor quibbles and then go ahead and merge please?
python/GafferTest/TestCase.py
Outdated
if self.__temporaryDirectory is not None : | ||
for root, dirs, files in os.walk( self.temporaryDirectory() ) : |
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.
Minor quibble - let's use __temporaryDirectory
here rather than temporaryDirectory()
, to match the other uses in this block. We're saying "the directory we made it earlier" rather than "make the directory".
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.
Sounds good, fixed in 2d73134
python/GafferTest/TestCase.py
Outdated
@@ -115,6 +115,9 @@ def tearDown( self ) : | |||
IECore.RefCounted.collectGarbage() | |||
|
|||
if self.__temporaryDirectory is not None : | |||
if os.name == "nt" : | |||
subprocess.check_call( [ "icacls", self.temporaryDirectory(), "/grant", "Users:(OI)(CI)(W)" ] ) |
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.
Same quibble again.
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.
Fixed in f0575ef
2df5d22
to
394d3fe
Compare
Great! Fixed up, squashed down and merging. |
This improves a few things with
TestCase.tearDown
:tearDown()
. I did a double check to make sure the call toicacls
doesn't increase the overall test times too much. It does increase the time relatively, but when testingGafferSceneTest
, it was adding up to only a bit over half a second.GafferImage.CatalogueTest.testNonWritableDirectory
on Windows because it was thought Windows couldn't make a directory truly read-only. Using extended permissions allows that, so this test can be adapted and enabled.Checklist