Skip to content

test: fix stale _DummyInner mock signatures in test_embeddings.py#120

Merged
dcfocus merged 1 commit into
lance-format:mainfrom
dcfocus:fix/test-embeddings-dummyinner-drift
Jun 28, 2026
Merged

test: fix stale _DummyInner mock signatures in test_embeddings.py#120
dcfocus merged 1 commit into
lance-format:mainfrom
dcfocus:fix/test-embeddings-dummyinner-drift

Conversation

@dcfocus

@dcfocus dcfocus commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes the stale _DummyInner mock in python/tests/test_embeddings.py whose add / upsert / search signatures had drifted from the real pyo3 bindings.

Context.add now passes 22 positional args to _inner.add (the bindings gained tenant, source, run_id, created_at, …), but the stub accepted at most 18 — so every add/upsert test failed at the call with:

TypeError: _DummyInner.add() takes from 10 to 18 positional arguments but 22 were given

…before it ever reached its assertion. These failures pre-dated and were independent of any feature work, and weren't surfaced in CI (the Python test workflow's path doesn't match how the suite runs locally).

Fix

Capture only the leading arguments the tests assert on (role, content, data_type, embedding, plus external_id for upsert) and absorb the rest with *args, **kwargs. The stub now won't drift again as binding signatures grow (e.g. the projection args added in #116).

Test-only change — no production code touched.

Result

  • pytest python/tests/test_embeddings.py15 passed (was 8 failing).
  • ruff format --check, ruff check, pyright — clean.

Independent of the open feature PRs; based on main.

The hand-written `_DummyInner` stub in `test_embeddings.py` had `add` /
`upsert` / `search` signatures that drifted from the real pyo3 bindings as
they gained parameters (`tenant`, `source`, `run_id`, `created_at`, and the
projection args). `Context.add` now passes 22 positional args to
`_inner.add`, but the stub accepted at most 18 — so every test that exercised
`add` / `upsert` failed with `TypeError: _DummyInner.add() takes from 10 to
18 positional arguments but 22 were given`, before reaching its assertion.

These failures predated and were independent of any feature change; they
weren't caught in CI because the Python test workflow's path doesn't match
how the suite is invoked locally.

Fix: capture only the leading arguments the tests assert on (`role`,
`content`, `data_type`, `embedding`, and `external_id` for `upsert`) and
absorb the rest with `*args, **kwargs`, so the stub no longer drifts when the
real binding signatures grow.

`test_embeddings.py`: 15 passed (was 8 failing). No production code changed.
@dcfocus dcfocus merged commit 42bcf59 into lance-format:main Jun 28, 2026
5 checks passed
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