Skip to content
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

bpo-34239: Convert test_bz2 to use tempfile #8485

Merged
merged 2 commits into from
Jul 26, 2018
Merged

bpo-34239: Convert test_bz2 to use tempfile #8485

merged 2 commits into from
Jul 26, 2018

Conversation

tjguk
Copy link
Member

@tjguk tjguk commented Jul 26, 2018

[Creating tentatively to see how the non-Windows tests run]

test_bz2 currently uses the test.support.TESTFN functionality which creates a temporary file local to the test directory named around the pid.

This can give rise to race conditions where tests are competing with each other to delete and recreate the file.

This change converts the tests to use tempfile.mkstemp which gives a different file every time from the system's temp area

https://bugs.python.org/issue34239

test_bz2 currently uses the test.support.TESTFN functionality which creates a temporary file local to the test directory named around the pid.

This can give rise to race conditions where tests are competing with each other to delete and recreate the file.

This change converts the tests to use tempfile.mkstemp which gives a different file every time from the system's temp area
Copy link
Member

@pfmoore pfmoore left a comment

Choose a reason for hiding this comment

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

LGTM. As an approach, this seems fine, although it's possible that a test that expects the file to not exist in advance could fail if changed to use this approach. That would be fairly easy to spot and fix on a case by case basis, though.

@tjguk
Copy link
Member Author

tjguk commented Jul 26, 2018

Thanks @pfmoore. I'll merge this one in. In fact, test_mmap over at #8486 does exactly that so I've had to operate a little differently. As you say: case by case.

@tjguk tjguk merged commit 6a62e1d into python:master Jul 26, 2018
@bedevere-bot
Copy link

@tjguk: Please replace # with GH- in the commit message next time. Thanks!

@tjguk tjguk deleted the Issue34329-test_bz2 branch July 28, 2018 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants