Skip to content

Conversation

@bmwiedemann
Copy link
Contributor

@bmwiedemann bmwiedemann commented Feb 5, 2018

Lib/test/support/__init__.py#L334 was using our mocked listdir to check when the deleted files were gone - and thus it thought they remained there

note: someone with MSWindows needs to test this fix

https://bugs.python.org/issue30693

@serhiy-storchaka
Copy link
Member

This makes the test passing. But mocking os.listdir which is used in a cleaning up code looks unsafe to me. This can left files when really something prevented from deleting them immediately (this is happened on Windows with an antivirus detector). It would be better to restore the original os.listdir before starting the cleaning up.

@bmwiedemann bmwiedemann force-pushed the tarfile branch 2 times, most recently from 74e3592 to 5c18489 Compare February 6, 2018 10:14
it was using our mocked listdir to check when the files were gone
@bmwiedemann
Copy link
Contributor Author

updated to only have the mocked version for tar.add

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Thanks, this LGTM!

@bedevere-bot
Copy link

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

@miss-islington
Copy link
Contributor

Thanks @bmwiedemann for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 6, 2018
it was using our mocked listdir to check when the files were gone.
(cherry picked from commit 4ad703b)

Co-authored-by: Bernhard M. Wiedemann <[email protected]>
@bedevere-bot
Copy link

GH-5567 is a backport of this pull request to the 3.7 branch.

serhiy-storchaka pushed a commit that referenced this pull request Feb 6, 2018
it was using our mocked listdir to check when the files were gone.
(cherry picked from commit 4ad703b)

Co-authored-by: Bernhard M. Wiedemann <[email protected]>
@bmwiedemann bmwiedemann deleted the tarfile branch February 7, 2018 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OS-windows skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants