CI: Fail when a test-gating dependency is installed nowhere - #67294
Open
jbrockmendel wants to merge 3 commits into
Open
CI: Fail when a test-gating dependency is installed nowhere#67294jbrockmendel wants to merge 3 commits into
jbrockmendel wants to merge 3 commits into
Conversation
pytest.importorskip and td.skip_if_no skip silently, so a dependency that is in no CI environment produces tests that never run anywhere. A local run cannot see it, because a development environment installs more than CI does. Add a pre-commit check that every module gated this way is installed in at least one environment the unit test workflow runs pytest in. It found ipython, which is only in the typing environment, so the nine tests behind the `ip` fixture have never run in CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZH7J4sFBwdKdbtuVdMysc
jbrockmendel
marked this pull request as ready for review
August 30, 2026 19:54
mroeschke
reviewed
Aug 30, 2026
Declare botocore explicitly rather than relying on it arriving with boto3, and drop the redundant toolz gate in test_dask -- toolz is a hard dependency of dask, so the extra importorskip never fired on its own. ALLOWED_UNDECLARED is now just moto, which CI serves from a container. Move ipython from its own feature into test-base, so the tests behind the `ip` fixture also run under minimum-versions, downstream, no-pyarrow and the nightly environments. Narrow the pre-commit trigger to the files that actually hold gates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Now that ipython is in test-base, test_publishes runs everywhere, and its latex assertion fails in the environments built from test-base alone -- numpy-nightly, pyarrow-nightly and py313-freethreading. `_repr_latex_` calls `to_latex`, which renders via Styler and so needs jinja2; without it the mimebundle has no text/latex key. Split that half into its own test behind an importorskip, so the table schema and html coverage keeps running in the minimal environments. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XhQUdt2tN1H4PFfPS3ahxm
mroeschke
approved these changes
Aug 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature. — CI/test-infra only, no user-visible change.pytest.importorskipandtd.skip_if_noskip silently. When a dependency is installed in no CI environment, the tests behind that gate never run anywhere, and nothing reports it: a local run can't see it either, because a development environment installs more than CI does. That's GH-26890, which has been open since 2019.This adds a pre-commit check that every module gated by
importorskip/skip_if_nounderpandas/is installed in at least one pixi environment thatunit-tests.ymlruns pytest in. Environments are read out of the workflow matrix (includinginclude:entries) rather than hardcoded, so it follows the workflow automatically.What it found
ipythonis declared only in[feature.typing.dependencies]. Confirmed inpixi.lock: thepy313environment resolves 379 packages, none of them ipython;typinghasipython-9.16.1. This isn't pixi-migration fallout — the pre-pixici/deps/actions-313.yamlhad no ipython either. It is inenvironment.ymlandrequirements-dev.txt, which is exactly why the gap was invisible to anyone running the suite locally.So the nine tests behind the
ipfixture (pandas/conftest.py) have never run in CI:io/formats/test_ipython_compat.pytest_publishes,test_publishes_not_implemented,test_enable_data_resource_formatterframe/test_api.pytest_tab_complete_warning(×2 params)arrays/categorical/test_warnings.pytest_tab_complete_warningindexes/test_base.pytest_tab_complete_warningresample/test_resampler_grouper.pytest_tab_complete_ipython6_warningio/formats/test_to_html.pytest_repr_html_ipython_configThe three in
test_ipython_compat.pyare the only coverage ofenable_data_resource_formatter/_repr_data_resource_inpandas/io/formats/printing.py.The fix adds
ipythonto[feature.test-base.dependencies], so it lands in every environment that runs the suite — not justpy311–py314, but alsominimum-versions,downstream,no-pyarrowand the nightlies. It solves cleanly everywhere, includingpy313-freethreading(ipython 9.17.0againstpython 3.13.15 cp313t). Thepixi.lockupdate is ipython's dependency tree, includingpsutil>=7, which conda-forge's ipython requires. All nine tests pass locally againstipython 9.17.0, the version the new lock resolves.Turning the tests on surfaced one latent failure, fixed here too.
test_publishesasserts atext/latexkey in the mimebundle, but_repr_latex_callsto_latex, which renders through Styler and so needs jinja2 — which the environments built fromtest-basealone (numpy-nightly,pyarrow-nightly,py313-freethreading) don't install. That half is now a separate test behind animportorskip, so the table-schema and html coverage keeps running in the minimal environments. It illustrates the point nicely: the assertion has been wrong for as long as it has existed, and no run anywhere could tell you.Allowlist
One module is gated but legitimately absent from
pixi.toml, with the reason in the source:moto, which CI serves from a container viaPANDAS_MOTO_URL.Two others started out on that list and shouldn't have been.
botocoreis now declared explicitly next toboto3rather than relying on it arriving transitively —pandas/io/common.pyimports it as a real optional dependency, so leaning on boto3's dependency tree is exactly the invisible coupling this check exists to catch. Thetoolzgate intest_daskis gone: toolz is a hard dependency of dask, so the extraimportorskipcould never fire on its own.Scope
This is deliberately the cheap half of GH-26890. It catches the failure at the point where someone forgets to add a dependency to the env files, costs no CI time, and needs no per-job flag plumbing. It does not catch a dependency that's declared but fails to install, or a skip caused by something other than a missing import.
Two things I'd rather raise than fold in here:
--strict-optional-deps(apytest_runtest_makereporthookwrapper turning import-caused skips into failures) would cover the declared-but-absent case. I prototyped it and it works, but it can only be enabled on the four full-dependency environments —no-pyarrow,minimum-versions,py313-freethreading,downstreamand the nightlies intentionally lack dependencies. Happy to open it separately if there's appetite.td.skip_if_not_us_localetestslocale.getlocale()[0] != "en_US", and the ubuntu matrix setsLANG: C.UTF-8(the two locale jobs setit_IT/zh_CN), so all 27 usages across 10 files skip on every ubuntu job. Same failure mode, no dependency involved.For context on how invisible this class is: a local full-suite run skips 7653 tests, of which only 47 are import-caused, and the ipython ones aren't among them — because ipython is installed locally.