[Python-Dev] Adding test.support.safe_rmpath()

[Brett Cannon]

On Thu, Feb 14, 2019 at 7:26 AM Giampaolo Rodola' <g.rodola at gmail.com>
wrote:

>
>
> On Thu, Feb 14, 2019 at 4:03 PM Tim Golden <mail at timgolden.me.uk> wrote:
>
>> On 14/02/2019 14:56, Giampaolo Rodola' wrote:
>> >
>> >
>> > On Thu, Feb 14, 2019 at 3:25 PM Eric Snow <ericsnowcurrently at gmail.com
>> > <mailto:ericsnowcurrently at gmail.com>> wrote:
>> >
>> >     On Thu, Feb 14, 2019, 02:47 Ronald Oussoren via Python-Dev
>> >     <python-dev at python.org <mailto:python-dev at python.org> wrote:
>> >
>> >
>> >         I usually use shutil.rmtree for tests that need to create
>> >         temporary files, and create a temporary directory for those
>> >         files (that is, use tempfile.mkdtemp in setUp() and use
>> >         shutil.rmtree in tearDown()). That way I don’t have to adjust
>> >         house-keeping code when I make changes to test code.
>> >
>> >
>> >     Same here.
>> >
>> >     -eric
>> >
>> >
>> > What I generally do is avoid relying on tempfile.mkdtemp() and always
>> > use TESTFN instead. I think it's cleaner as a pradigm because it's an
>> > incentive to not pollute the single unit tests with
>> `self.addCleanup()`
>> > instructions (the whole cleanup logic is always supposed to occur in
>> > setUp/tearDown):
>>
>> Must chime in here because I've been pushing (variously months & years
>> ago) to move *away* from TESTFN because it generates numerous
>> intermittent errors on my Windows setup. I've had several goes at
>> starting to do that but a combination of my own lack of time plus some
>> people's reluctance to go that route altogether has stalled the thing.
>>
>> I'm not sure I understand the difference in cleanup/teardown terms
>> between using tempfile and using TESTFN. The objections I've seen from
>> people (apart, obviously, from test churn) are to do with building up
>> testing temp artefacts on a possibly low-sized disk.
>>
>> TJG
>>
>
> I suppose you mean the intermittent failures are usually due to "file is
> already in use by another process" correct? test.support's unlink(),
> rmdir() and rmtree() functions already implement a retry-with-timeout logic
> in order to prevent this issue. I suppose when this issue may still occur,
> though, is when the file/handle is held by another process, meaning that
> the unit-test probably forgot to terminate()/wait() a subprocess or should
> have used support.read_children(). In summary, my approach is more "strict"
> because it implies that unit-tests always do a proper cleanup.
> tempfile.mkdtemp() may prevent failures but it may hide a unit-test which
> doesn't do a proper file/dir cleanup and should have been fixed instead.
> The drawback in practical terms is that orphaned test files are left behind.
>
> Extra: an argument in favor of using tempfile.mkdtemp() instead of TESTFN
> is parallel testing, but I think we're not using it.
>

With -j you can do parallel testing and I know I always run with that on.
But TESTFN does *attempt *to account for that
<https://github.com/python/cpython/blob/8a1657b93469580ef345c7c91738587f3d76e87d/Lib/test/support/__init__.py#L835>
by using the PID in the name.

Having said that, I do use tempfile like Eric, Ronald, and Tim when I write
tests as I have real-world experience using tempfile so I usually remember
to clean up versus TESTFN which is Python stdlib internals only and I have
to remember that it won't clean up itself.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20190214/4ba76fcd/attachment.html>