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

Make parallel tests insensitive to user configuration #8727

Merged
merged 9 commits into from
May 23, 2019
5 changes: 5 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ jobs:
- image: quay.io/pypa/manylinux1_i686
steps:
- checkout
- run:
name: Write configuration items to standard location to make sure they are ignored in parallel mode
command: |
mkdir -p $HOME/.astropy/config/
printf "unicode_output = True\nmax_width = 500" > $HOME/.astropy/config/astropy.cfg
- run:
name: Install dependencies for Python 3.5
command: /opt/python/cp35-cp35m/bin/pip install numpy scipy pytest pytest-astropy pytest-xdist Cython jinja2
Expand Down
6 changes: 6 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,12 @@ before_install:
# Check CC variable
- echo $CC

# Write configuration items to standard location to make sure they are
# ignored (the tests will fail if not)
- mkdir -p $HOME/.astropy/config/
- printf "unicode_output = True\nmax_width = 500" > $HOME/.astropy/config/astropy.cfg


install:
- git clone git://github.com/astropy/ci-helpers.git
- source ci-helpers/travis/setup_conda.sh
Expand Down
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,9 @@ Other Changes and Additions
time squared (division already correctly resulted in a dimensionless
``Quantity``). [#8356]

- Made running the tests insensitive to local user configuration when running
the tests in parallel mode or directly with pytest. [#8727]

Installation
^^^^^^^^^^^^

Expand Down
26 changes: 26 additions & 0 deletions astropy/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
(i.e. those that would not necessarily be shared by affiliated packages
making use of astropy's test runner).
"""
import os
import builtins
import tempfile
from importlib.util import find_spec

import astropy
Expand Down Expand Up @@ -44,6 +46,20 @@ def pytest_configure(config):
matplotlibrc_cache.update(matplotlib.rcParams)
matplotlib.rcdefaults()

# Make sure we use temporary directories for the config and cache
# so that the tests are insensitive to local configuration. Note that this
# is also set in the test runner, but we need to also set it here for
# things to work properly in parallel mode

builtins._xdg_config_home_orig = os.environ.get('XDG_CONFIG_HOME')
builtins._xdg_cache_home_orig = os.environ.get('XDG_CACHE_HOME')

os.environ['XDG_CONFIG_HOME'] = tempfile.mkdtemp('astropy_config')
os.environ['XDG_CACHE_HOME'] = tempfile.mkdtemp('astropy_cache')

os.mkdir(os.path.join(os.environ['XDG_CONFIG_HOME'], 'astropy'))
os.mkdir(os.path.join(os.environ['XDG_CACHE_HOME'], 'astropy'))


def pytest_unconfigure(config):
builtins._pytest_running = False
Expand All @@ -52,6 +68,16 @@ def pytest_unconfigure(config):
matplotlib.rcParams.update(matplotlibrc_cache)
matplotlibrc_cache.clear()

if builtins._xdg_config_home_orig is None:
os.environ.pop('XDG_CONFIG_HOME')
else:
os.environ['XDG_CONFIG_HOME'] = builtins._xdg_config_home_orig

if builtins._xdg_cache_home_orig is None:
os.environ.pop('XDG_CACHE_HOME')
else:
os.environ['XDG_CACHE_HOME'] = builtins._xdg_cache_home_orig

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed here but not in the other conftest files ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how I feel about playing with env var in general in conftest. Let's say user sets custom vars, then run tests, conftest messes them up, now user complains that the these vars are not what they set anymore. Can this happen?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason it's needed here and not in the other conftest.py files is because this is the only one which might be used inside a persistent python session such as IPython. The other ones will only matter if you run pytest, and the Python process will end at the end of the tests. Any changes to os.environ don't change the variables beyond the Python process:

Python 3.7.1 (v3.7.1:260ec2c36a, Oct 20 2018, 03:13:28) 
[Clang 6.0 (clang-600.0.57)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> os.environ['FOO'] = 'BAR'
>>> 
$ echo $FOO
$

However, in this specific conftest.py file I should probably indeed save any pre-existing variables and reset then in unconfigure.


PYTEST_HEADER_MODULES['Cython'] = 'cython'
PYTEST_HEADER_MODULES['Scikit-image'] = 'skimage'
9 changes: 7 additions & 2 deletions astropy/tests/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,13 @@ def run_tests(self, **kwargs):
else:
plugins = []

# override the config locations to not make a new directory nor use
# existing cache or config
# Override the config locations to not make a new directory nor use
# existing cache or config. Note that we need to do this here in
# addition to in conftest.py - for users running tests interactively
# in e.g. IPython, conftest.py would get read in too late, so we need
# to do it here - but at the same time the code here doesn't work when
# running tests in parallel mode because this uses subprocesses which
# don't know about the temporary config/cache.
astropy_config = tempfile.mkdtemp('astropy_config')
astropy_cache = tempfile.mkdtemp('astropy_cache')

Expand Down
20 changes: 20 additions & 0 deletions conftest.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,25 @@
# Licensed under a 3-clause BSD style license - see LICENSE.rst

# This file is the main file used when running tests with pytest directly,
# in particular if running e.g. ``pytest docs/``.

import os
import tempfile

pytest_plugins = [
'astropy.tests.plugins.display',
]

# Make sure we use temporary directories for the config and cache
# so that the tests are insensitive to local configuration.

os.environ['XDG_CONFIG_HOME'] = tempfile.mkdtemp('astropy_config')
os.environ['XDG_CACHE_HOME'] = tempfile.mkdtemp('astropy_cache')

os.mkdir(os.path.join(os.environ['XDG_CONFIG_HOME'], 'astropy'))
os.mkdir(os.path.join(os.environ['XDG_CACHE_HOME'], 'astropy'))

# Note that we don't need to change the environment variables back or remove
# them after testing, because they are only changed for the duration of the
# Python process, and this configuration only matters if running pytest
# directly, not from e.g. an IPython session.
23 changes: 23 additions & 0 deletions docs/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Licensed under a 3-clause BSD style license - see LICENSE.rst

# This file needs to be included here to make sure commands such
# as ``python setup.py test ... -t docs/...`` works, since this
# will ignore the conftest.py file at the root of the repository
# and the one in astropy/conftest.py

import os
import tempfile

# Make sure we use temporary directories for the config and cache
# so that the tests are insensitive to local configuration.

os.environ['XDG_CONFIG_HOME'] = tempfile.mkdtemp('astropy_config')
os.environ['XDG_CACHE_HOME'] = tempfile.mkdtemp('astropy_cache')

os.mkdir(os.path.join(os.environ['XDG_CONFIG_HOME'], 'astropy'))
os.mkdir(os.path.join(os.environ['XDG_CACHE_HOME'], 'astropy'))

# Note that we don't need to change the environment variables back or remove
# them after testing, because they are only changed for the duration of the
# Python process, and this configuration only matters if running pytest
# directly, not from e.g. an IPython session.