Skip to content

bpo-39986: Make test_listdir from test_os more robust #19035

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MatzeB
Copy link
Contributor

@MatzeB MatzeB commented Mar 17, 2020

The test_listdir test of test_os assumed that calling listdir on the
root directory twice gives the same results, however this can fail if
unrelated process create files in the root directory in between the two
calls. This changes the test to create a temporary directory with two
files inside and call listdir on this temporary directory instead.

https://bugs.python.org/issue39986

The test_listdir test of test_os assumed that calling listdir on the
root directory twice gives the same results, however this can fail if
unrelated process create files in the root directory in between the two
calls. This changes the test to create a temporary directory with two
files inside and call listdir on this temporary directory instead.
with tempfile.TemporaryDirectory() as tmpdir:
os.chdir(tmpdir)
open("a.txt", "w").close()
open("test_file.foo", "w").close()
Copy link
Member

Choose a reason for hiding this comment

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

Can you write something in the files just to make sure they are created on all platforms?

os.chdir(tmpdir)
open("a.txt", "w").close()
open("test_file.foo", "w").close()
self.assertEqual(set(os.listdir()), set(os.listdir(tmpdir)))
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to check that the file you created are in the output?

Comment on lines +2209 to +2213
with tempfile.TemporaryDirectory() as tmpdir:
os.chdir(tmpdir)
open("a.txt", "w").close()
open("test_file.foo", "w").close()
self.assertEqual(set(os.listdir()), set(os.listdir(tmpdir)))
Copy link
Member

Choose a reason for hiding this comment

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

There is already a directory with a known content: self.dir.

Suggested change
with tempfile.TemporaryDirectory() as tmpdir:
os.chdir(tmpdir)
open("a.txt", "w").close()
open("test_file.foo", "w").close()
self.assertEqual(set(os.listdir()), set(os.listdir(tmpdir)))
os.chdir(self.dir)
self.assertEqual(set(os.listdir()), expected)

@serhiy-storchaka serhiy-storchaka added needs backport to 3.13 bugs and security fixes and removed needs backport to 3.11 only security fixes labels May 9, 2024
@hugovk hugovk removed the needs backport to 3.12 only security fixes label Apr 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review needs backport to 3.13 bugs and security fixes skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants