-
-
Notifications
You must be signed in to change notification settings - Fork 22
Fix compatibility with pytest-run-parallel and add free-threaded CI jobs #63
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
Changes from 2 commits
3e219cf
340e541
afaae64
d6fc9dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) | ||
|
|
@@ -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): | ||
|
|
||
|
|
@@ -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: | ||
|
|
@@ -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): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't this implementation now exactly equivalent to the default ?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
| yield | ||
| return | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| # 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] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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') | ||
|
astrofrog marked this conversation as resolved.
Outdated
|
||
|
|
||
| # Generate the reference file first | ||
| code = subprocess.call( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't this use the builtin It does require pytest>=6.2, which is very reasonable now IMO (6.2 came out in 2020)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| ['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 | ||
There was a problem hiding this comment.
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
314there:tableswill ever support itThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done