test.regrtest should complain if a test doesn't remove temporary files

[vstinner]

BPO	22390
Nosy	@pitrou, @vstinner, @ezio-melotti, @voidspace, @serhiy-storchaka
Files	regrtest_warn_lost_files.patch fix_tests.patch regrtest_warn_lost_files2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2015-03-30.07:10:41.991>
created_at = <Date 2014-09-11.14:30:52.008>
labels = ['type-feature', 'tests']
title = "test.regrtest should complain if a test doesn't remove temporary files"
updated_at = <Date 2015-03-30.08:41:08.553>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2015-03-30.08:41:08.553>
actor = 'vstinner'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2015-03-30.07:10:41.991>
closer = 'serhiy.storchaka'
components = ['Tests']
creation = <Date 2014-09-11.14:30:52.008>
creator = 'vstinner'
dependencies = []
files = ['36601', '36647', '36894']
hgrepos = []
issue_num = 22390
keywords = ['patch']
message_count = 23.0
messages = ['226777', '226786', '226787', '226788', '226792', '226793', '227015', '228584', '229196', '229200', '229234', '236080', '239488', '239525', '239527', '239529', '239532', '239533', '239534', '239536', '239539', '239575', '239582']
nosy_count = 6.0
nosy_names = ['pitrou', 'vstinner', 'ezio.melotti', 'michael.foord', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue22390'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

[vstinner]

A change in test_glob of issue bpo-13968 started to fail because a previous test created temporary files but didn't remove them.

test.regrtest should at least emit a warning if the temporary directory used to run tests is not empty before removing it.

[serhiy-storchaka]

Here is a patch. It warns if new files or directories are left after test run in current directory and removes those of them which starts with TESTFN (i.e. TESTFN_UNICODE, TESTFN + "2", TESTFN + ".py" and other names used in tests).

[vstinner]

I would prefer to fix tests instead of trying to remove arbitrary files. I'm not sure that "fn.startswith(support.TESTFN)" check is safe enough.

Maybe we should not remove TESTFN neither. You are supposed to be able to run tests without regrtest.

[vstinner]

I ran the test suite with the patch applied:

7 tests altered the execution environment:
test_imp test_import test_pdb test_posix test_source_encoding
test_support test_threaded_import

I will try to investigate these warnings.

[serhiy-storchaka]

I would prefer to fix tests instead of trying to remove arbitrary files. I'm
not sure that "fn.startswith(support.TESTFN)" check is safe enough.

This allow other tests which leaks the same file to be reported too.

support.TESTFN contains process ID so it is unique enough.

[serhiy-storchaka]

I will try to investigate these warnings.

Run tests with the -vv option.

[vstinner]

fix_tests.patch: Fix the 7 tests which create files without removing them. 4 tests (test_imp, test_pdb, test_source_encoding and test_support) create a __pycache__ directory. Maybe this directory should be ignored by regrtest?

[python-dev]

New changeset 8cf8bff3569e by Victor Stinner in branch '3.4':
Issue bpo-22390: Remove files created by tests
https://hg.python.org/cpython/rev/8cf8bff3569e

[serhiy-storchaka]

How about regrtest_warn_lost_files.patch?

[vstinner]

This allow other tests which leaks the same file to be reported too.

I don't understand your answer. The "fn.startswith(support.TESTFN)" test is not used in get_files().

If I understood correctly, your patch changes two things:

• it now reports files created for tests but not deleted (purpose of this issue)
• it removes more files than before

I don't understand why you want to remove more files than before. You may open a different issue, or at least explain the rationale.

I never see any forgotten test file after running tests, so I don't see why you are worried because of them. And with your first patch, we will now noticed forgotten files, so we can just fix tests.

[serhiy-storchaka]

I don't understand why you want to remove more files than before. You may
open a different issue, or at least explain the rationale.

I thought it would be good idea slightly extend this cleanup while we are
here. I'm not motivated enough to open a different issue.

Well, here is a patch which removes only TESTFN. It is still improved, uses
support.unlink and support.rmtree instead of os.unlink and shutil.rmtree.

You can just drop cleanup code at all if you prefer. All is good to me.

I never see any forgotten test file after running tests, so I don't see why
you are worried because of them.

This is because regrtest creates temporary directory and goes to it. But when
you execute Python test directly, test files are created in the current
directory.

And with your first patch, we will now
noticed forgotten files, so we can just fix tests.

But we will noticed only one about test at the time if several tests forgot
the same file. This will needed several iterations.

[serhiy-storchaka]

What about this issue?

[serhiy-storchaka]

Victor?