Re: [Python-Dev] Tests failing on Windows with TESTFN

2018-07-28 Thread Jeremy Kloth
On Sat, Jul 28, 2018 at 7:15 PM Chris Jerdonek  wrote:
> Regardless of whether the tempfile or TESTFN approach is used, I think it 
> would be best for a few reasons if the choice is abstracted behind a uniquely 
> named test function (e.g. make_test_file if not already used).

+1, although my particular choice of color would be to add a pair of
functions, mkstemp and mkdtemp, to match the style of
test.support-wrapped library functions for use in the test harness.

-- 
Jeremy Kloth
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Tests failing on Windows with TESTFN

2018-07-28 Thread Chris Jerdonek
On Sat, Jul 28, 2018 at 5:40 PM Brett Cannon  wrote:

>
> On Sat, Jul 28, 2018, 15:13 eryk sun,  wrote:
>
>> On Sat, Jul 28, 2018 at 9:17 PM, Jeremy Kloth 
>> wrote:
>> >
>> > *PLEASE*, don't use tempfile to create files/directories in tests.  It
>> > is unfriendly to (Windows) buildbots.  The current approach of
>> > directory-per-process ensures no test turds are left behind, whereas
>> > the tempfile solution slowly fills up my buildbot.  Windows doesn't
>> > natively clean out the temp directory.
>>
>> FYI, Windows 10 storage sense (under system->storage) can be
>> configured to delete temporary files on a schedule. Of course that
>> doesn't help with older systems.
>>
>
> If Windows doesn't clean up its temp directory on a regular basis then
> that doesn't suggest to me not to use tempfile, but instead that the use of
> tempfile still needs to clean up after itself. And if there is a lacking
> feature in tempfile then we should add it instead of a avoiding the module.
>

Regardless of whether the tempfile or TESTFN approach is used, I think it
would be best for a few reasons if the choice is abstracted behind a
uniquely named test function (e.g. make_test_file if not already used).

—Chris



> -Brett
>
> ___
>> Python-Dev mailing list
>> Python-Dev@python.org
>> https://mail.python.org/mailman/listinfo/python-dev
>>
> Unsubscribe:
>> https://mail.python.org/mailman/options/python-dev/brett%40python.org
>>
> ___
> Python-Dev mailing list
> Python-Dev@python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/chris.jerdonek%40gmail.com
>
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Tests failing on Windows with TESTFN

2018-07-28 Thread Jeremy Kloth
On Sat, Jul 28, 2018 at 6:43 PM Brett Cannon  wrote:
> If Windows doesn't clean up its temp directory on a regular basis then that 
> doesn't suggest to me not to use tempfile, but instead that the use of 
> tempfile still needs to clean up after itself. And if there is a lacking 
> feature in tempfile then we should add it instead of a avoiding the module.

Mind you, this is mentioned in the confines of the test harness where
just about anything can happen (and usually does!).  Something that
cannot be coded against using just tempfile is cleanup on process
abort.  The per-process-directory approach handles this case.

I would think it is desired to have no leftovers after running the
test harness (especially in regards to the buildbots).

Now, I'm not sure the exact cause of all of the leftovers in the TEMP
directory, but it is definitely something that is currently happening
(and shouldn't be).  It is not exactly the easiest of tasks to track
the file usage of every test in the test suite.  It is certainly
easier to replace usages of os.unlink with test.support.unlink within
the test suite.

-- 
Jeremy Kloth
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Tests failing on Windows with TESTFN

2018-07-28 Thread Brett Cannon
On Sat, Jul 28, 2018, 15:13 eryk sun,  wrote:

> On Sat, Jul 28, 2018 at 9:17 PM, Jeremy Kloth 
> wrote:
> >
> > *PLEASE*, don't use tempfile to create files/directories in tests.  It
> > is unfriendly to (Windows) buildbots.  The current approach of
> > directory-per-process ensures no test turds are left behind, whereas
> > the tempfile solution slowly fills up my buildbot.  Windows doesn't
> > natively clean out the temp directory.
>
> FYI, Windows 10 storage sense (under system->storage) can be
> configured to delete temporary files on a schedule. Of course that
> doesn't help with older systems.
>

If Windows doesn't clean up its temp directory on a regular basis then that
doesn't suggest to me not to use tempfile, but instead that the use of
tempfile still needs to clean up after itself. And if there is a lacking
feature in tempfile then we should add it instead of a avoiding the module.

-Brett

___
> Python-Dev mailing list
> Python-Dev@python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/brett%40python.org
>
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] [Python-checkins] bpo-34239: Convert test_bz2 to use tempfile (#8485)

2018-07-28 Thread Chris Jerdonek
On Thu, Jul 26, 2018 at 2:05 PM, Tim Golden  wrote:
> https://github.com/python/cpython/commit/6a62e1d365934de82ff7c634981b3fbf218b4d5f
> commit: 6a62e1d365934de82ff7c634981b3fbf218b4d5f
> branch: master
> author: Tim Golden 
> committer: GitHub 
> date: 2018-07-26T22:05:00+01:00
> summary:
>
> bpo-34239: Convert test_bz2 to use tempfile (#8485)
>
> * bpo-34239: Convert test_bz2 to use tempfile
>
> test_bz2 currently uses the test.support.TESTFN functionality which creates a 
> temporary file local to the test directory named around the pid.
>
> This can give rise to race conditions where tests are competing with each 
> other to delete and recreate the file.

Per the other thread--
https://mail.python.org/pipermail/python-dev/2018-July/154762.html
this seems like a wrong statement of the problem as tests are properly
cleaning up after themselves. The leading hypothesis is that unrelated
Windows processes are delaying the deletion (e.g. virus scanners).

--Chris

>
> This change converts the tests to use tempfile.mkstemp which gives a 
> different file every time from the system's temp area
>
> files:
> M Lib/test/test_bz2.py
>
> diff --git a/Lib/test/test_bz2.py b/Lib/test/test_bz2.py
> index 003497f28b16..e62729a5a2f8 100644
> --- a/Lib/test/test_bz2.py
> +++ b/Lib/test/test_bz2.py
> @@ -6,6 +6,7 @@
>  import os
>  import pickle
>  import glob
> +import tempfile
>  import pathlib
>  import random
>  import shutil
> @@ -76,11 +77,14 @@ class BaseTest(unittest.TestCase):
>  BIG_DATA = bz2.compress(BIG_TEXT, compresslevel=1)
>
>  def setUp(self):
> -self.filename = support.TESTFN
> +fd, self.filename = tempfile.mkstemp()
> +os.close(fd)
>
>  def tearDown(self):
> -if os.path.isfile(self.filename):
> +try:
>  os.unlink(self.filename)
> +except FileNotFoundError:
> +pass
>
>
>  class BZ2FileTest(BaseTest):
>
> ___
> Python-checkins mailing list
> python-check...@python.org
> https://mail.python.org/mailman/listinfo/python-checkins
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Tests failing on Windows with TESTFN

2018-07-28 Thread eryk sun
On Sat, Jul 28, 2018 at 9:17 PM, Jeremy Kloth  wrote:
>
> *PLEASE*, don't use tempfile to create files/directories in tests.  It
> is unfriendly to (Windows) buildbots.  The current approach of
> directory-per-process ensures no test turds are left behind, whereas
> the tempfile solution slowly fills up my buildbot.  Windows doesn't
> natively clean out the temp directory.

FYI, Windows 10 storage sense (under system->storage) can be
configured to delete temporary files on a schedule. Of course that
doesn't help with older systems.
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Tests failing on Windows with TESTFN

2018-07-28 Thread eryk sun
On Sat, Jul 28, 2018 at 5:20 PM, Tim Golden  wrote:
>
> I've got a mixture of Permission (winerror 13) & Access errors (winerror 5)

EACCES (13) is a CRT errno value. Python raises PermissionError for
EACCES and EPERM (1, not used). It also does the reverse mapping for
WinAPI calls, so PermissionError is raised either way. About 25 WinAPI
error codes map to EACCES. Commonly it's due to either
ERROR_ACCESS_DENIED (5) or ERROR_SHARING_VIOLATION (32).

open() uses read-write sharing but not delete sharing. In this case
trying to either delete an already open file or open a file that's
already open with delete access (e.g. an O_TEMPORARY open) both fail
with a sharing violation.

An access-denied error could be due to a range of causes. Over 20
NTAPI status codes map to ERROR_ACCESS_DENIED. Commonly for a file
it's due to one of the following status codes:

STATUS_ACCESS_DENIED (0xc022)
The file security doesn't grant the requested access
to the caller.

STATUS_DELETE_PENDING (0xc056)
The file's delete disposition is set, i.e. it's flagged to be
deleted when the last handle is closed. Opening a new
handle is disallowed for any access.

STATUS_FILE_IS_A_DIRECTORY (0xc0ba)
Except when using backup semantics, CreateFile calls
NtCreateFile with the flag FILE_NON_DIRECTORY_FILE,
so only non-directory files/devices can be opened.

STATUS_CANNOT_DELETE (0xc121)
The file is either readonly or memory mapped.
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Tests failing on Windows with TESTFN

2018-07-28 Thread Chris Jerdonek
On Sat, Jul 28, 2018 at 10:20 AM, Tim Golden  wrote:
>
> Here's the thing. The TESTFN approach creates a directory per process
> test_python_ and some variant of @test__tmp inside that directory.

I filed an issue some years back about this (still open):
https://bugs.python.org/issue15305
The pid is unnecessarily being used twice. Once the directory is
created for the process, there shouldn't be a need to disambiguate
within the directory further.

> But the same filename in the same directory is used for every test in that
> process. Now, leaving aside the particular mechanism by which Windows
> processes might be holding locks which prevent removal or re-addition, that
> already seems like an odd choice.
>
> I think that was my starting point: rather than develop increasingly
> involved and still somewhat brittle mechanisms, why not do what you'd
> naturally do with a new test and use tempfile? I was expecting someone to
> come forward to highlight some particular reason why the TESTFN approach is
> superior, but apart from a reference to the possibly cost of creating a new
> temporary file per test, no-one really has.

I think there is value in having the files used during test runs in a
known location. How about a middle ground where, once the
process-specific directory is created in a known location, unique
files are created within that directory (e.g. using a random suffix,
or a perhaps a suffix that is a function of the test name / test id)?

This would address both the issue you're experiencing, and, if the
directory is deleted at the end of the test run, it would address the
issue Jeremy mentions of growing leftover files. There could also be a
check at the end of each test run making sure that the directory is
empty (to check for tests that aren't cleaning up after themselves).

--Chris
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Tests failing on Windows with TESTFN

2018-07-28 Thread Jeremy Kloth
On Sat, Jul 28, 2018 at 11:20 AM Tim Golden  wrote:
> Although things have moved on since that discussion and
> test.support.unlink has grown some extra legs, all it's done really is
> to push the bump along the carpet for a bit. I've got a newly-installed
> Win10 machine with the typical MS Antivirus & TortoiseGitCache vying for
> locks with every other process. I've just done a full test run:
>
> python -mtest -j0 -v > test.log

I, for one, would like to see that log.  The issues you are have are
fairly unique.  Just check out the buildbot status page.  I know that
some of the workers are somewhat limited, but my worker
(https://buildbot.python.org/all/#/workers/12) is running on dedicated
hardware.  Before https://bugs.python.org/issue15496 was applied, the
errors you describe were indeed happening, but no longer.

> Here's the thing. The TESTFN approach creates a directory per process
> test_python_ and some variant of @test__tmp inside that
> directory. But the same filename in the same directory is used for every
> test in that process. Now, leaving aside the particular mechanism by
> which Windows processes might be holding locks which prevent removal or
> re-addition, that already seems like an odd choice.

Well, since every test (well, test file) is run in its own process, a
directory per process doesn't seem that odd.  You seem to be focusing
on 2 tests in particular, both of which have not been converted to use
test.support.unlink for removing.  The "trick" with
test.support.unlink now is that it waits until the file has truly
removed from the directory before continuing.  Using this, in turn,
would mean that following create *should* succeed without error.  If
test.support.unlink returns without warning, the removed file is
really gone.

> I think that was my starting point: rather than develop increasingly
> involved and still somewhat brittle mechanisms, why not do what you'd
> naturally do with a new test and use tempfile? I was expecting someone
> to come forward to highlight some particular reason why the TESTFN
> approach is superior, but apart from a reference to the possibly cost of
> creating a new temporary file per test, no-one really has.

*PLEASE*, don't use tempfile to create files/directories in tests.  It
is unfriendly to (Windows) buildbots.  The current approach of
directory-per-process ensures no test turds are left behind, whereas
the tempfile solution slowly fills up my buildbot.  Windows doesn't
natively clean out the temp directory.

Additionally, I'm working on a solution to allow the test machinery to
use a given directory for test outputs to allow for a RAM-backed
filesystem for temporary files.  With this, I've had the full test
suite (with -uall including bigmem) take ~5min.  Having some tests use
the Windows default test directory would break this.  It is simply not
feasible to have the entire Windows TEMP in RAM, but just Python's
test suite usage is small enough (<6Gb).

Another point, some tests require that the temporary filename resides
on the same drive as the source directory (discovered in developing
the above).  This means that, Windows development would be restricted
to the same drive as the user directory, if the per-directory approach
isn't used.

-- 
Jeremy Kloth
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Tests failing on Windows with TESTFN

2018-07-28 Thread Tim Golden

On 28/07/2018 17:27, Jeremy Kloth wrote:

On Sat, Jul 28, 2018 at 8:41 AM Tim Golden  wrote:

1) Why are these errors occurring? ie are we dodging a root cause issue


The root cause is how Windows handles file deletions.  When a file is
removed, it is not immediately removed from the directory, instead, it
is simply marked for deletion.  Only once all open handles to the file
are closed, is it removed from the directory.  The most common cause
of additional open handles to a file is a antivirus scanner.


Thanks, Jeremy. I'm already aware of that. (If you look back at 
https://bugs.python.org/issue7443 you'll find me explaining the same to 
MvL some years ago). [In case the tone isn't clear, there's absolutely 
no sarcasm implied here. I just wanted to make it clear that I'm at 
least partly au fait with the situation :)].


Although things have moved on since that discussion and 
test.support.unlink has grown some extra legs, all it's done really is 
to push the bump along the carpet for a bit. I've got a newly-installed 
Win10 machine with the typical MS Antivirus & TortoiseGitCache vying for 
locks with every other process. I've just done a full test run:


python -mtest -j0 -v > test.log

and I've got a mixture of Permission (winerror 13) & Access errors 
(winerror 5). The former are usually attempting to open a TESTFN file; 
the latter are attempting to unlink one. Most of the Error 5 are 
os.unlink, but at least one is from test.support.unlink.


I understand the unable-to-recreate (virus checkers with share-delete 
handles) but I'm not so clear on what's blocking the unlink. IOW I think 
we have more than one issue.


Here's the thing. The TESTFN approach creates a directory per process 
test_python_ and some variant of @test__tmp inside that 
directory. But the same filename in the same directory is used for every 
test in that process. Now, leaving aside the particular mechanism by 
which Windows processes might be holding locks which prevent removal or 
re-addition, that already seems like an odd choice.


I think that was my starting point: rather than develop increasingly 
involved and still somewhat brittle mechanisms, why not do what you'd 
naturally do with a new test and use tempfile? I was expecting someone 
to come forward to highlight some particular reason why the TESTFN 
approach is superior, but apart from a reference to the possibly cost of 
creating a new temporary file per test, no-one really has.



If you are not seeing the RuntimeWarnings, then something else is amiss.


I think I've seen one RuntimeWarning in the many, many times I've been 
running through tests today.


TJG
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Tests failing on Windows with TESTFN

2018-07-28 Thread Jeremy Kloth
On Sat, Jul 28, 2018 at 8:41 AM Tim Golden  wrote:
> 1) Why are these errors occurring? ie are we dodging a root cause issue

The root cause is how Windows handles file deletions.  When a file is
removed, it is not immediately removed from the directory, instead, it
is simply marked for deletion.  Only once all open handles to the file
are closed, is it removed from the directory.  The most common cause
of additional open handles to a file is a antivirus scanner.

> 2) Isn't this what test.support.unlink is there to solve?

Yes.  If test.support.unlink doesn't succeed in removing the file, a
RuntimeWarning will be emitted.

> For (1) I'm putting together a test run using code which I originally
> wrote for https://bugs.python.org/issue7443 to force the issues out into
> the open.

These intermittent errors are tough as it is really just a matter of
timing. The faster the disk and the more loaded the CPU, the more
often these can occur, however.

> For (2), yes: test.support.unlink is supposed to solve that. But it's
> either not doing enough retries etc. or it's missing a trick.

If you are not seeing the RuntimeWarnings, then something else is amiss.

-- 
Jeremy Kloth
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


[Python-Dev] USE_STACKCHECK and running out of stack

2018-07-28 Thread Ronald Oussoren via Python-Dev
Hi,

I’m looking at PyOS_CheckStack because this feature might be useful on macOS 
(and when I created bpo-33955 for this someone ran with it and created a patch).

Does anyone remember why the interpreter raises MemoryError and not 
RecursionError when PyOS_CheckStack detects that we’re about to run out of 
stack space? 

The reason I’m looking into this is that the default stack size on macOS is 
fairly small and I’d like to avoid crashing the interpreter when running out of 
stackspace on threads created by the system (this is less of a risk on threads 
created by Python itself because we can arrange for a large enough stack in 
that case).

Ronald

___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Tests failing on Windows with TESTFN

2018-07-28 Thread Tim Golden

On 25/07/2018 16:07, Tim Golden wrote:
One problem is that certain tests use support.TESTFN (a local directory 
constructed from the pid) for output files etc. However this can cause 
issues on Windows when recreating the folders / files for multiple 
tests, especially when running in parallel.


Here's an example on my laptop deliberately running 3 tests with -j0 
which I know will generate an error about one time in three:


C:\work-in-progress\cpython>python -mtest -j0 test_urllib2 test_bz2 
test_importlib




[... snip ...]


[...] I wanted to ask first
whether there was any objection to my converting tests to using tempfile 
functions which should avoid the problem?


Thanks to those who responded here and/or on the two bpo issues I've 
already raised:


https://bugs.python.org/issue34239 -- test_bz2
https://bugs.python.org/issue34240 -- test_mmap

Two key questions have been posed in different ways:

1) Why are these errors occurring? ie are we dodging a root cause issue

2) Isn't this what test.support.unlink is there to solve?

For (1) I'm putting together a test run using code which I originally 
wrote for https://bugs.python.org/issue7443 to force the issues out into 
the open.


For (2), yes: test.support.unlink is supposed to solve that. But it's 
either not doing enough retries etc. or it's missing a trick.


To be clear: my motivation here isn't some housekeeping or modernisation 
exercise. I want to get a clear test run on a fresh build on Windows. At 
present I never get that. I'd be interested to hear from other Windows 
devs whether they have the same experience?


TJG
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com