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 test to confirm that rotation handler is working as intended. #8197
Conversation
|
||
def test_getFilesToDelete(self): | ||
temp_dir = tempfile.mkdtemp() | ||
file_handle, log_file = tempfile.mkstemp(suffix=".txt", dir=temp_dir) |
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's a pitty we don't have TemporaryDirectory in Python 2.7, but, for the purpose of this code, could it be done using with tempfile.TemporaryFile() as log_file
, so you don't need to import and use shutil?
i.e.
@@ -1,5 +1,4 @@
import os
-import shutil
import tempfile
from time import sleep
@@ -33,21 +32,14 @@ class KolibriTimedRotatingFileHandlerTestCase(TestCase):
self.assertNotEqual(os.listdir(archive_dir), [])
def test_getFilesToDelete(self):
- temp_dir = tempfile.mkdtemp()
- file_handle, log_file = tempfile.mkstemp(suffix=".txt", dir=temp_dir)
- os.close(file_handle)
- handler = KolibriTimedRotatingFileHandler(log_file, backupCount=3, when="s")
- sleep(1)
- handler.doRollover()
- sleep(1)
- handler.doRollover()
- sleep(1)
- handler.doRollover()
- sleep(1)
- handler.doRollover()
- self.assertEqual(len(handler.getFilesToDelete()), 1)
- try:
- os.remove(log_file)
- except OSError:
- pass
- shutil.rmtree(temp_dir, ignore_errors=True)
+ with tempfile.NamedTemporaryFile(suffix=".txt") as log_file:
+ handler = KolibriTimedRotatingFileHandler(log_file.name, backupCount=3, when="s")
+ sleep(1)
+ handler.doRollover()
+ sleep(1)
+ handler.doRollover()
+ sleep(1)
+ handler.doRollover()
+ sleep(1)
+ handler.doRollover()
+ self.assertEqual(len(handler.getFilesToDelete()), 1)
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.
No, I've had to remove our use of NamedTemporaryFile
in general, because it breaks on Windows due to permissions errors.
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.
oh, I see it can't be used as the file is not closed so the directory can't be created. There's no alternative for Python 2.7 without tempfile.TemporaryDirectory
Summary
References
Provides a test to show that #5989 is not a real issue and so closes #5989
Reviewer guidance
Does the test make sense?
Testing checklist
PR process
Reviewer checklist
yarn
andpip
)