Skip to content

test: turn unexpected warnings into errors - #2480

Merged
schloerke merged 3 commits into
mainfrom
schloerke/fix-test-warnings
Sep 1, 2026
Merged

test: turn unexpected warnings into errors#2480
schloerke merged 3 commits into
mainfrom
schloerke/fix-test-warnings

Conversation

@schloerke

@schloerke schloerke commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

The unit suite was emitting 7 warnings. This clears them and adds
filterwarnings = error to pytest.ini so new ones fail the suite instead of
scrolling past.

What was warning

  • render.download deprecation (3 tests). Three test_renderer.py
    validation tests still decorated with the deprecated render.download; they
    now use render.download_button. The deprecation itself is still covered by
    test_render_download_is_deprecated.
  • Offcanvas accessibility (2 tests). The two anonymous-panel
    show_offcanvas() tests legitimately trigger the "should have an
    aria-label" warning — show_offcanvas() on bare content has no way to set
    a title — so they now assert it with pytest.warns() rather than leak it.
  • polars cat.get_categories() (2 tests). Not ours to fix — see below.

The narwhals bug this turned up

Chasing the last warning found a real upstream problem, filed as
narwhals-dev/narwhals#3895.

serialize_dtype() in shiny/render/_data_frame_utils/_tbl_data.py calls
col.cat.get_categories() to send a categorical column's levels to the client.
narwhals' polars cat namespace is a pure passthrough — class PolarsSeriesCatNamespace(PolarsSeriesNamespace, PolarsCatNamespace): ..., an
empty body dispatching via __getattr__ — so that lands verbatim on polars'
own Series.cat.get_categories(), deprecated as of polars 1.44 and documented
for removal in polars 2.0. It isn't only test noise: any app rendering a
polars categorical column prints this DeprecationWarning to its user's
console
, and there's nothing the app author can do about it.

I prototyped a fix in _tbl_data.py and then threw it away, because every
version of it was polars-specific, which defeats the point of going through
narwhals at all:

  • Branching on col.implementation.is_polars() to read the native series works,
    but puts backend-specific code in a module whose whole job is being
    backend-agnostic.
  • col.unique(maintain_order=True) is backend-agnostic but wrong: pandas
    categoricals carry declared categories, including unused ones and in
    declared order, and two existing test cases cover exactly that.
  • dtype.categories — what polars' own deprecation message recommends — raises
    AttributeError: 'narwhals.stable.v1._dtypes.Enum' object has no attribute '_cached_categories', because narwhals/stable/v1/_dtypes.py skips
    NwEnum.__init__. We import narwhals.stable.v1, so the documented
    workaround is unavailable to us. That's noted as a secondary item on the
    issue.

So the upstream issue is the fix, with a scoped ignore in pytest.ini holding
the line until it lands. It carries a runnable reprex, its real output, a
suggested implementation for the polars backend (Enum → pl.Series(dtype.categories),
Categorical → unique(maintain_order=True).drop_nulls().cast(pl.String), both
verified to return today's values), and an offer to send the PR.

Note on filterwarnings = error

The tradeoff: a new pandas/polars/starlette release that adds a
DeprecationWarning will turn CI red on an unrelated PR. The fix is one ignore
line, and the red is usually a real signal — as the narwhals case shows, a
warning in our test output was a warning in users' app consoles. Every ignore
carries a comment so the list stays prunable.

CI turned up a second, more awkward class, which the later commits here ignore:

  • ResourceWarning from a leaked TemporaryDirectory (failed the oldest-deps
    job, blamed on test_theme_css_compiles_and_is_cached).
  • PytestUnraisableExceptionWarning wrapping ValueError: <Token ...> was created in a different Context from an abandoned
    ExtendedTask._done_callback coroutine (failed 3.12/macOS, blamed on
    test_otel_reactive_execution.py).

Both are raised from finalizers at GC time, so they fail whichever test happens
to be running when the collector fires rather than the test at fault — flaky
rather than strict, so they're ignored as a class. Warnings raised on a real
call stack are still fatal, which is where the value is.

The ExtendedTask one looks like a genuine latent bug (a context token reset
from the wrong context during teardown), currently invisible because it only
happens in an abandoned coroutine. Out of scope here; worth its own issue.

No library code changes; test config and tests only.

The unit suite emitted 7 warnings. Rather than let them accumulate, set
`filterwarnings = error` in pytest.ini so a stray warning fails the suite,
and clear out the existing ones:

* `test_renderer.py` used the deprecated `render.download` for three
  validation tests; they now use `render.download_button`. The deprecation
  itself is still covered by `test_render_download_is_deprecated`.
* The two anonymous-panel `show_offcanvas()` tests legitimately trigger the
  accessibility warning (there is no way to give an anonymous panel a
  title), so they now assert it with `pytest.warns()`.

The one remaining warning is not ours to fix: narwhals passes
`cat.get_categories()` straight through to polars, where it is deprecated.
Filed upstream as narwhals-dev/narwhals#3895 and ignored by message in
pytest.ini until that lands.
A leaked `TemporaryDirectory` is finalized whenever the collector runs, so as
an error it fails whichever test happens to be running at that moment (in CI,
`test_theme_css_compiles_and_is_cached` on the oldest-deps job) rather than the
test that leaked it. Under `error` the warning is raised inside the finalizer,
becomes an unraisable exception, and pytest reports it as a
`PytestUnraisableExceptionWarning`.
The 3.12/macOS job hit the same GC-attribution problem from a different angle:
a `ValueError` from an abandoned `ExtendedTask._done_callback` coroutine, which
pytest surfaces as `PytestUnraisableExceptionWarning` against whichever test was
running at collection time (`test_otel_reactive_execution.py` here). Warnings
raised from finalizers can't be attributed to the code at fault, so as errors
they make the suite flaky rather than strict.
@schloerke
schloerke merged commit 885506b into main Sep 1, 2026
176 checks passed
@schloerke
schloerke deleted the schloerke/fix-test-warnings branch September 1, 2026 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant