Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/ci_workflows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ jobs:
- windows: py312-test-pytest74
- linux: py313-test-pytest83
- linux: py313-test-pytest90
- linux: py313-test-parallel
- linux: py313t-test
- linux: py313t-test-parallel
Comment on lines +34 to +35
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd suggest going straight to 314t here:

  • 3.13t was always experimental
  • cibuildwheel is currently sunsetting it so it seems unlikely that tables will ever support it

- linux: py313-test-devdeps
publish:
needs: tests
Expand Down
20 changes: 14 additions & 6 deletions pytest_arraydiff/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,12 +240,18 @@ def wrap_array_interceptor(plugin, item):
# Only intercept array on marked array tests
if item.get_closest_marker('array_compare') is not None:

# Guard against wrapping more than once (e.g. when pytest-run-parallel
# runs the same item multiple times).
if getattr(item.obj, '_arraydiff_wrapped', False):
return

# Use the full test name as a key to ensure correct array is being retrieved
test_name = generate_test_name(item)

def array_interceptor(store, obj):
def wrapper(*args, **kwargs):
store.return_value[test_name] = obj(*args, **kwargs)
wrapper._arraydiff_wrapped = True
return wrapper

item.obj = array_interceptor(plugin, item.obj)
Expand All @@ -260,6 +266,10 @@ def __init__(self, config, reference_dir=None, generate_dir=None, default_format
self.default_format = default_format
self.return_value = {}

def pytest_collection_modifyitems(self, items):
for item in items:
wrap_array_interceptor(self, item)

@pytest.hookimpl(hookwrapper=True)
def pytest_runtest_call(self, item):

Expand Down Expand Up @@ -298,8 +308,6 @@ def pytest_runtest_call(self, item):

baseline_remote = reference_dir.startswith('http')

# Run test and get array object
wrap_array_interceptor(self, item)
yield
test_name = generate_test_name(item)
if test_name not in self.return_value:
Expand Down Expand Up @@ -372,11 +380,11 @@ def __init__(self, config):
self.config = config
self.return_value = {}

@pytest.hookimpl(hookwrapper=True)
def pytest_runtest_call(self, item):

if item.get_closest_marker('array_compare') is not None:
def pytest_collection_modifyitems(self, items):
for item in items:
wrap_array_interceptor(self, item)

@pytest.hookimpl(hookwrapper=True)
def pytest_runtest_call(self, item):
Comment on lines +387 to +388
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

isn't this implementation now exactly equivalent to the default ?

yield
return
6 changes: 5 additions & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,15 @@ install_requires =
pytest>=5.0
numpy

# tables limitation is until 3.9.3 is out as that supports ARM OSX.
# tables limitation is until 3.9.3 is out as that supports ARM OSX, and
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this additional comment is particularly helpful here; it'd be much more fitting in tox.ini

# tables wheels are not yet available for free-threaded CPython. Since
# PEP 508 has no marker for that, tables is split into its own extra and
# tox only installs it on the non-free-threaded envs.
[options.extras_require]
test =
astropy
pandas
test_hdf5 =
tables;platform_machine!='arm64'

[options.entry_points]
Expand Down
37 changes: 37 additions & 0 deletions tests/test_pytest_arraydiff.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,3 +177,40 @@ def test_single_reference(self, spam):

def test_nofile():
pass


TEST_PARALLEL = """
import pytest
import numpy as np
from astropy.io import fits
@pytest.mark.array_compare(file_format='fits')
def test_parallel():
return fits.PrimaryHDU(np.arange(3 * 5).reshape((3, 5)).astype(np.int64))
"""


def test_parallel_iterations():
"""Regression test: arraydiff should work with pytest-run-parallel."""
pytest.importorskip('pytest_run_parallel')

tmpdir = tempfile.mkdtemp()
test_file = os.path.join(tmpdir, 'test.py')
with open(test_file, 'w') as f:
f.write(TEST_PARALLEL)

gen_dir = os.path.join(tmpdir, 'reference')
Comment on lines +196 to +201
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suggest using the tmp_path builtin fixture here

Suggested change
tmpdir = tempfile.mkdtemp()
test_file = os.path.join(tmpdir, 'test.py')
with open(test_file, 'w') as f:
f.write(TEST_PARALLEL)
gen_dir = os.path.join(tmpdir, 'reference')
test_file = tmp_path / 'test.py'
test_file.write_test(TEST_PARALLEL)
gen_dir = tmp_path / 'reference'


# Generate the reference file first
code = subprocess.call(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't this use the builtin pytester fixture instead ? https://docs.pytest.org/en/stable/reference/reference.html#std-fixture-pytester

It does require pytest>=6.2, which is very reasonable now IMO (6.2 came out in 2020)

['pytest', f'--arraydiff-generate-path={gen_dir}', test_file],
timeout=30,
)
assert code == 0

# Now run with --arraydiff and multiple iterations
code = subprocess.call(
['pytest', '--arraydiff', f'--arraydiff-reference-path={gen_dir}',
'--parallel-threads=2', '--iterations=3', test_file],
timeout=30,
)
assert code == 0
8 changes: 8 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
[tox]
envlist =
py{39,310,311,312,313,314}-test{,-pytestoldest,-pytest52,-pytest53,-pytest60,-pytest61,-pytest62,-pytest70,-pytest71,-pytest72,-pytest73,-pytest74,-devdeps}
py{313t,314t}-test{,-parallel}
codestyle
isolated_build = true

[testenv]
changedir = .tmp/{envname}
setenv =
py313t,py314t: PYTHON_GIL = 0
devdeps: PIP_EXTRA_INDEX_URL = https://pypi.anaconda.org/astropy/simple https://pypi.anaconda.org/liberfa/simple https://pypi.anaconda.org/scientific-python-nightly-wheels/simple
description = run tests
deps =
Expand All @@ -24,13 +26,19 @@ deps =
pytest80: pytest==8.0.*
pytest83: pytest==8.3.*
pytest90: pytest==9.0.*
parallel: pytest-run-parallel
devdeps: git+https://github.com/pytest-dev/pytest#egg=pytest
devdeps: numpy>=0.0.dev0
devdeps: pandas>=0.0.dev0
devdeps: pyerfa>=0.0.dev0
devdeps: astropy>=0.0.dev0
extras =
test
# tables (PyTables) is needed for the pandas/HDF5 file_format, but
# has no free-threaded wheels and its sdist requires libhdf5-dev,
# which CI does not install. Skip the HDF5 extras on free-threaded
# envs.
Comment on lines +39 to +40
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aligning the phrasing with the way configuration is written

Suggested change
# which CI does not install. Skip the HDF5 extras on free-threaded
# envs.
# which CI does not install. Only run these test on GIL-enabled builds.

!py313t-!py314t: test_hdf5
commands =
# Force numpy-dev after something in the stack downgrades it
devdeps: python -m pip install --pre --upgrade --extra-index-url https://pypi.anaconda.org/scientific-python-nightly-wheels/simple numpy
Expand Down
Loading