Skip to content

Conversation

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Dec 28, 2025

Comment on lines +6390 to +6393
try:
self.assertFalse(rh.shouldRollover(self.next_rec()))
finally:
rh.close()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, can we use self.addCleanup(rh.close) or

with contextlib.closing(rh):
    self.assertFalse(rh.shouldRollover(self.next_rec()))

to simplify this test code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.addCleanup(rh.close) will not work -- it should be closed before exiting the with block, otherwise the thread will hang.

I did not use contextlib.closing() only because contextlib is not yet imported. The difference is not so large. Maybe I'll do this at the end.

@serhiy-storchaka serhiy-storchaka added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 29, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit cc46702 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F143259%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants