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

Killed unit tests can leave a sample_project folder in the top-level folder #589

Closed
edreamleo opened this issue Dec 11, 2022 · 7 comments · Fixed by #596
Closed

Killed unit tests can leave a sample_project folder in the top-level folder #589

edreamleo opened this issue Dec 11, 2022 · 7 comments · Fixed by #596
Labels
bug Unexpected or incorrect user-visible behavior

Comments

@edreamleo
Copy link
Contributor

edreamleo commented Dec 11, 2022

This has happened several times, but my notes don't tell me how to reproduce the bug.

Hmm. I bet I know what happened. I probably killed the console running the tests, thereby randomly preventing a tearDown method from executing.

testutils.sample_project special-cases posix platforms. Perhaps this bug only bites on Windows. Or perhaps projects could be created in a tmp folder.

Investigation shows that many setUp and tearDown methods create and destroy the sample_projects folder. It's non-trivial to verify that each tearDown method destroys the folder created by setUp.

So it might be safer to define a ProjectTestCase class whose setUp and tearDown methods call testutils.sample_project and testutils.remove_project respectively. Subclasses would then be free of managing the project folder themselves.

@edreamleo edreamleo added the bug Unexpected or incorrect user-visible behavior label Dec 11, 2022
@edreamleo edreamleo changed the title Failing unit tests can leave a sample_project folder in the top-level folder Killed unit tests can leave a sample_project folder in the top-level folder Dec 11, 2022
@lieryan
Copy link
Member

lieryan commented Dec 12, 2022

This never happens in Linux because in Linux, the leftover sample_project() is actually left in /tmp or /dev/shm, not in the current directory.

We should update sample_project() to use either the stdlib's tempfile.TemporaryDirectory() or pytest's tmpdir fixture. Either of which should make Windows' also use Windows-style temporary directory mechanism.

I think tempfile.TemporaryDirectory() is going to be easier to implement since sample_project() can be used on legacy unittests that doesn't use pytest' fixture mechanisms.

@edreamleo
Copy link
Contributor Author

@lieryan Another alternative: add sample_project to .gitignore.

lieryan added a commit that referenced this issue Dec 14, 2022
lieryan added a commit that referenced this issue Dec 14, 2022
Fix #589 Cleanup testutils.sample_project()
@lieryan
Copy link
Member

lieryan commented Dec 14, 2022

This should now be fixed in #596, @edreamleo would you be able to test this on Windows?

@edreamleo
Copy link
Contributor Author

@lieryan Thanks!

@edreamleo
Copy link
Contributor Author

@lieryan I ran and interrupted pytest several times on Windows. The sample_project folder never persisted.

But this doesn't seem like much of a test :-) Do you have any ideas about how to strengthen the test?

@lieryan
Copy link
Member

lieryan commented Dec 14, 2022

Thanks. That should be good enough. Just keep an eye and report back if the issues ever returns.

@edreamleo
Copy link
Contributor Author

@lieryan will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected or incorrect user-visible behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants