classification
Title: sys.path in tests contains system directories
Type: behavior Stage: needs patch
Components: Build, Tests Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, ajaksu2, brett.cannon, carljm, doko, dugan, eli.bendersky, eric.araujo, eric.snow, loewis, martin.panter, ncoghlan, nyb, orsenthil, pitrou, r.david.murray, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2007-03-06 01:37 by nyb, last changed 2016-05-16 04:13 by martin.panter.

Files
File name Uploaded Description Edit
make_test_S.patch r.david.murray, 2010-12-31 19:02 review
python-2.7-issue1674555.patch Arfrever, 2012-12-31 08:53
python-3.2-issue1674555.patch Arfrever, 2012-12-31 08:54
python-3.3-issue1674555.patch Arfrever, 2012-12-31 08:54
python-3.4-issue1674555.patch Arfrever, 2012-12-31 08:54
python-2.7-issue1674555.v2.patch martin.panter, 2016-05-16 04:13 review
Messages (19)
msg31436 - (view) Author: Theodoros V. Kalamatianos (nyb) Date: 2007-03-06 01:37
While trying to debug a testsuite crash, I discovered that a faulty module from the system wide directories was being picked up. The module in question was sgmlop.so, which was being imported by Lib/xmlrpclib.py. (BTW isn't it considered bad form for a module within the Python distribution to load a third party module?)

In my opinion, the main issue here is that while testing a Python build, no out-of-tree modules should be able to be imported in any case, in order to avoid pollution of the testing environment. Yet AFAICT, there is no place where the sys.path is manipulated to remove the out-of-tree directories. I believe that the proper thing to do would be to allow only the build tree module directories in the search path.

Any comments/patches would be appreciated.


Best Regards,

Theodoros Kalamatianos
msg84675 - (view) Author: Daniel Diniz (ajaksu2) (Python triager) Date: 2009-03-30 22:08
Theodoros,
Can you provide a short script that reproduces this?
msg107216 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * (Python triager) Date: 2010-06-06 19:42
I noticed the same problem in many tests.
E.g. test___all__ fails when pyxml is installed.
msg124889 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-12-29 23:56
One way to "fix" this would be to have make test run the tests with -j1 and pass in the -S and -s flags, and then have regrtest special case test_site and remove those flags for the run of that single test.

An interesting facet of this proposal in that it actually isolates the tests better.  But it will also slow down the test suite run a bit.  Since the tests are already run twice by make test, that may not matter all that much (that is, it's a start-the-run-and-walk-away situation anyway).

Note that all tests except test_trace currently pass with -S -s.
msg124972 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-12-31 19:02
Here is a proof of concept patch if anyone wants to play with it.    Note that a higher value could be used for the j option; multiple threads help even on uniprocessor systems since a bunch of the tests spend time waiting around.

The patch removes the '-l' option from the make test run.  I'm not sure that matters, since we do our best to fix memory leaks before shipping a release, and we don't use make test to do our leak testing (as far as I know).

I'm not sure this is an appropriate solution, so I won't apply this unless I get some favorable votes from committers and/or packagers. 

One interesting thing is that several additional tests show up as altering the execution environment when run this way.  That bears investigation at some point, but is an orthogonal issue.

As noted, test_trace does not pass, but only one test within it fails, so that ought to be easy enough to fix.  Also note that this "fix" would only be applicable to 3.2 and 2.7.
msg124993 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2011-01-01 09:53
I find the solution (running every test in a subprocess) a bit too drastic for the problem. How about a modified approach: run regrtest with -S, and have it create a subprocess for test_site only?
msg125202 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * (Python triager) Date: 2011-01-03 17:37
Apparently this patch isn't sufficient for test___all__.
Please create empty _xmlplus directory (without __init__.py) in site-packages directory appropriate for given sys.prefix (e.g. /usr/lib/python3.2/site-packages/_xmlplus for sys.prefix='/usr').

The output of `make test` contains:
LD_LIBRARY_PATH=/var/tmp/portage/dev-lang/python-3.2_pre20110102/work/python-3.2_pre20110102: ./python -Wd -E -bb -S -s  ./Lib/test/regrtest.py -j1 
== CPython 3.2b2+ (py3k:87612, Jan 3 2011, 18:30:27) [GCC 4.4.5]
==   Linux-2.6.34-tuxonice-r8-AFTA-x86_64-Intel-R-_Pentium-R-_Dual_CPU_T2390_@_1.86GHz-with-gentoo-2.0.1 little-endian
==   /var/tmp/portage/dev-lang/python-3.2_pre20110102/work/python-3.2_pre20110102/build/test_python_2301
[  1/349] test_grammar
[  2/349] test_opcodes
[  3/349] test_dict
[  4/349] test_builtin
[  5/349] test_exceptions
[  6/349] test_types
[  7/349] test_unittest
[  8/349] test_doctest

[  9/349] test_doctest2

[ 10/349] test___all__

Warning -- sys.path was modified by test___all__
test test___all__ failed -- Traceback (most recent call last):
  File "/var/tmp/portage/dev-lang/python-3.2_pre20110102/work/python-3.2_pre20110102/Lib/test/test___all__.py", line 101, in test_all
    self.check_all(modname)
  File "/var/tmp/portage/dev-lang/python-3.2_pre20110102/work/python-3.2_pre20110102/Lib/test/test___all__.py", line 28, in check_all
    raise FailedImport(modname)
  File "/var/tmp/portage/dev-lang/python-3.2_pre20110102/work/python-3.2_pre20110102/Lib/contextlib.py", line 35, in __exit__
    next(self.gen)
  File "/var/tmp/portage/dev-lang/python-3.2_pre20110102/work/python-3.2_pre20110102/Lib/test/support.py", line 623, in _filterwarnings
    raise AssertionError("unhandled warning %s" % reraise[0])
AssertionError: unhandled warning {message : ImportWarning("Not importing directory '/usr/lib/python3.2/site-packages/_xmlplus': missing __init__.py",), category : 'ImportWarning', filename : '/var/tmp/portage/dev-lang/python-3.2_pre20110102/work/python-3.2_pre20110102/Lib/xml/__init__.py', lineno : 26, line : None}
[ 11/349] test___future__
[ 12/349] test__locale
[ 13/349] test_abc
msg125209 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-03 18:11
I agree with Martin here.
Also, since test_site is a special case here, perhaps it could arrange to launch its test cases in a subprocess? I don't think complicating regrtest is the most maintainable approach.
msg125215 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2011-01-03 19:00
The java/openjdk tests allow setting an attribute `samevm' for running a specific test.  maybe something like this could be used for some problematic tests which occasionally hang on some buildds?
msg125220 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-01-03 19:37
Yeah, making a generic way to put specific tests into a subprocess run sounds like a better solution.

But...

The xml error in test___all__ is due to the fact that test___all__ imports site.  So even with the above we'd need to special case site in test___all__.  This probably answers the question of why more tests show as changing the environment when run with -S -s, so there are probably other tests that would need to be touched as well.

This is becoming a bit of a jerry-rig.  Anyone have a better idea?  A way to identify the "system paths" (or, conversely, identify the paths *not* added by site) and delete them in regrtest before running the tests might be cleaner.

Or another idea: change site so that it does not execute on import, but instead the machinery that currently imports test site runs a 'setup' function after it does the import.  If test_site were then to test the setup function and take care to restore sys.path when it was done, I think things would work as expected if all tests were run with -S -s.  Of course, this would break all the custom site.py's out there, so it is probably a non-starter of an idea.

In the meantime, test___all__ could perhaps be made more robust in the face of import errors/warnings for those few modules that import from external libraries (xml, logging...anything else?)
msg125221 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-03 19:43
Le lundi 03 janvier 2011 à 19:37 +0000, R. David Murray a écrit :
> Or another idea: change site so that it does not execute on import,
> but instead the machinery that currently imports test site runs a
> 'setup' function after it does the import.

I'm not sure this means anything :) Importing a module automatically
entails running its top-level code (that's the only way it can create
functions, classes, etc.).

However, I agree that in the default site.py, we could stop calling
main() at the top-level and instead call it from the importing code.

> Of course, this would break all the custom site.py's out there, so it
> is probably a non-starter of an idea.

Are there many of them? And usually, you would take the original site.py
and modify it a little. Starting from scratch sounds crazy.

> In the meantime, test___all__ could perhaps be made more robust in the
> face of import errors/warnings for those few modules that import from
> external libraries (xml, logging...anything else?)

Well, ideally, these modules should stop importing external libraries
implicitly. Give them a dedicated function for that instead, that the
user can call if they want to.
msg125224 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-01-03 20:03
Ah, I hadn't looked closely enough at site.py to realize that the init work was being done by a 'main()' call.  Given that, just moving the main call out should be relatively unlikely to break any custom site.py.  Worse case would presumably be main() getting called twice, which could be annoying but probably not catastrophic.  One would like to think that anyone customizing site.py would redo the customization for a major release and would notice. (And no, I don't know how common customized site.py's are.)

Logging already only does its import if you construct an NTEventLogHandler, so that's probably safe enough.  Changing xml to require a function call might make some people mad, and other people happy...

I wonder if there are any other stdlib modules that do these kinds of external imports.  I suppose we can just fix xml and wait for additional bug reports :)

I'm not sure either of these changes (site.py-call or xml-activate-xmlplus) should be backported, though, assuming we go ahead with them.
msg127337 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * (Python triager) Date: 2011-01-28 20:28
I'm attaching the patch, which works for me.
- New, private variable (_PYTHONNOSITEPACKAGES) disables addition of site-packages directories to sys.path.
- regrtest.py always runs test_site.py in a subprocess.
msg132212 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-03-26 02:32
Note that site’s behavior is a bit different in 3.3 thanks to #11591.  See http://docs.python.org/dev/library/site#site.main
msg136552 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * (Python triager) Date: 2011-05-22 18:18
The advantage of an environment variable is that it is by default inherited by subprocesses.

(These patches no longer apply cleanly. I will create updated patches.)
msg179125 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-01-05 15:09
Issue #16817 discusses a similar problem arising with test___all__. The patches have to be updated to provide a generic way to delegate certain tests to separate subprocesses.

Arfrever, it's not necessary to update all the patches at once. Start with the one for 3.2 and after we figure out it's acceptable, it should be easy to convert to the other branches.
msg221992 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-06-30 23:26
@Arfrever will you create updated patches as stated in msg136552 ?
msg222012 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * (Python triager) Date: 2014-07-01 05:59
I update my patches every time they stop applying, but I do not feel that it is necessary to frequently attach new patches here.
msg265666 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-05-16 04:13
I did a quick merge of Arfrever’s 2.7 patch with current code (mainly pgo flags). Unfortunately it didn’t fix my problem with unhandled ImportWarning, but here it is in case it is useful.
History
Date User Action Args
2016-05-16 04:13:21martin.pantersetfiles: + python-2.7-issue1674555.v2.patch

messages: + msg265666
2016-01-20 11:27:43BreamoreBoysetnosy: - BreamoreBoy
2016-01-20 09:39:57serhiy.storchakasetnosy: + martin.panter, serhiy.storchaka

versions: + Python 3.6, - Python 3.4
2014-07-01 05:59:27Arfreversetmessages: + msg222012
2014-06-30 23:26:19BreamoreBoysetnosy: + BreamoreBoy

messages: + msg221992
versions: + Python 3.5, - Python 3.2, Python 3.3
2013-01-25 23:13:49eric.snowsetnosy: + eric.snow
2013-01-05 15:09:35eli.benderskysetmessages: + msg179125
versions: + Python 3.4, - Python 3.1
2013-01-05 15:07:34eli.benderskylinkissue16817 dependencies
2012-12-31 20:25:16orsenthilsetnosy: + orsenthil
2012-12-31 15:32:12ncoghlansetnosy: + ncoghlan
2012-12-31 08:54:59Arfreversetfiles: - python-2.7-issue1674555.patch
2012-12-31 08:54:50Arfreversetfiles: - python-3.2-issue1674555.patch
2012-12-31 08:54:39Arfreversetfiles: + python-3.4-issue1674555.patch
2012-12-31 08:54:26Arfreversetfiles: + python-3.3-issue1674555.patch
2012-12-31 08:54:14Arfreversetfiles: + python-3.2-issue1674555.patch
2012-12-31 08:53:52Arfreversetfiles: + python-2.7-issue1674555.patch
2012-12-30 23:31:55eli.benderskysetnosy: + eli.bendersky
2011-05-22 18:18:50Arfreversetmessages: + msg136552
2011-03-26 02:32:29eric.araujosetnosy: + brett.cannon, carljm

messages: + msg132212
versions: + Python 3.3
2011-01-28 20:29:44Arfreversetfiles: + python-2.7-issue1674555.patch
nosy: loewis, doko, pitrou, nyb, ajaksu2, dugan, eric.araujo, Arfrever, r.david.murray
2011-01-28 20:28:51Arfreversetfiles: + python-3.2-issue1674555.patch
nosy: loewis, doko, pitrou, nyb, ajaksu2, dugan, eric.araujo, Arfrever, r.david.murray
messages: + msg127337
2011-01-03 20:03:21r.david.murraysetnosy: loewis, doko, pitrou, nyb, ajaksu2, dugan, eric.araujo, Arfrever, r.david.murray
messages: + msg125224
2011-01-03 19:43:44pitrousetnosy: loewis, doko, pitrou, nyb, ajaksu2, dugan, eric.araujo, Arfrever, r.david.murray
messages: + msg125221
2011-01-03 19:37:31r.david.murraysetnosy: loewis, doko, pitrou, nyb, ajaksu2, dugan, eric.araujo, Arfrever, r.david.murray
messages: + msg125220
2011-01-03 19:00:45dokosetnosy: loewis, doko, pitrou, nyb, ajaksu2, dugan, eric.araujo, Arfrever, r.david.murray
messages: + msg125215
2011-01-03 18:11:56pitrousetnosy: loewis, doko, pitrou, nyb, ajaksu2, dugan, eric.araujo, Arfrever, r.david.murray
messages: + msg125209
2011-01-03 17:37:11Arfreversetnosy: loewis, doko, pitrou, nyb, ajaksu2, dugan, eric.araujo, Arfrever, r.david.murray
messages: + msg125202
2011-01-01 09:53:18loewissetnosy: loewis, doko, pitrou, nyb, ajaksu2, dugan, eric.araujo, Arfrever, r.david.murray
messages: + msg124993
2010-12-31 19:02:50r.david.murraysetfiles: + make_test_S.patch

nosy: + pitrou, doko, loewis
messages: + msg124972

keywords: + patch
2010-12-29 23:56:08r.david.murraysetnosy: nyb, ajaksu2, dugan, eric.araujo, Arfrever, r.david.murray
versions: - Python 2.6
messages: + msg124889
stage: test needed -> needs patch
2010-08-27 16:39:00r.david.murraysetnosy: + r.david.murray
2010-07-31 21:08:13eric.araujosetnosy: + eric.araujo
2010-06-06 19:42:34Arfreversetnosy: + Arfrever
title: Python 2.5 testsuite sys.path contains system dirs -> sys.path in tests contains system directories
messages: + msg107216

versions: + Python 3.1, Python 2.7, Python 3.2
2009-03-31 20:53:47dugansetnosy: + dugan
2009-03-30 22:08:34ajaksu2settype: behavior
components: + Tests
versions: + Python 2.6, - Python 2.5
nosy: + ajaksu2

messages: + msg84675
stage: test needed
2007-03-06 01:37:07nybcreate