Skip to content

Add edge-case tests for cross-table lookup (#136)#142

Merged
amc-corey-cox merged 1 commit intocross-table-lookupfrom
cross-table-lookup-test-coverage
Mar 9, 2026
Merged

Add edge-case tests for cross-table lookup (#136)#142
amc-corey-cox merged 1 commit intocross-table-lookupfrom
cross-table-lookup-test-coverage

Conversation

@turbomam
Copy link
Copy Markdown
Member

@turbomam turbomam commented Mar 9, 2026

Summary

Adds 11 tests covering edge cases in the LookupIndex and transform_spec engine introduced in #136. All tests pass on the cross-table-lookup branch — these document and lock in existing behavior.

Why these tests

Test area What it covers Why it matters
Duplicate keys LIMIT 1 returns a row when multiple rows share a key Current behavior is silent first-match without ORDER BY. Tests document this so any future change to query ordering is caught.
Empty secondary tables Headers-only TSV registers and queries cleanly Real-world data pipelines encounter empty tables; verifies graceful null propagation.
LookupIndex lifecycle close() clears state, post-close ops raise, double-close is safe Ensures the cleanup contract is stable for callers.
Engine no-joins regression transform_spec works when class_derivation has no joins: block The common case (no joins) must not be broken by the join machinery. No existing test covers this path through transform_spec.
Mixed derivations Joins and non-joins class_derivation blocks coexist in one spec Verifies that join table registration/cleanup per block doesn't interfere with subsequent non-join blocks.

Test files

  • tests/test_utils/test_lookup_index_edge_cases.py — 7 tests for LookupIndex
  • tests/test_transformer/test_engine_edge_cases.py — 4 tests for transform_spec

Note on duplicate-key semantics

The test_duplicate_keys_returns_a_row test intentionally does NOT assert which duplicate row is returned — it only verifies that a row comes back and that the key matches. The docstring documents that DuckDB's storage order makes this deterministic in practice, but the API doesn't guarantee it. If you'd prefer to make this a documented constraint (e.g., "first row wins") or add a warning/error for non-unique keys, I'm happy to adjust the tests accordingly.

Test plan

  • All 11 new tests pass on cross-table-lookup
  • All 14 existing join/lookup tests still pass
  • No changes to production code

🤖 Generated with Claude Code

Cover four gaps in test coverage for the LookupIndex and
transform_spec engine introduced in PR #136:

- Duplicate keys: verify LIMIT 1 first-match semantics and document
  that the returned row is non-deterministic for non-unique keys
- Empty secondary tables: headers-only TSV files register and query
  cleanly, returning None on lookup
- LookupIndex lifecycle: close() clears tables, operations after
  close() raise, double-close is safe
- Engine no-joins regression: transform_spec works correctly when
  class_derivations have no joins block (common case)
- Mixed derivations: joins and non-joins class_derivations coexist

All 11 tests pass on the cross-table-lookup branch.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds targeted edge-case tests to document and lock in existing behavior for the DuckDB-backed LookupIndex and the transform_spec engine introduced with cross-table lookup support.

Changes:

  • Add LookupIndex edge-case tests covering duplicate keys, empty tables, and lifecycle behavior after close().
  • Add transform_spec edge-case tests covering no-joins derivations, empty joined tables, and mixed (joins + non-joins) derivations in one spec.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/test_utils/test_lookup_index_edge_cases.py Adds 7 tests for duplicate-key semantics, empty secondary tables, and post-close() behavior of LookupIndex.
tests/test_transformer/test_engine_edge_cases.py Adds 4 tests to prevent regressions in transform_spec for no-joins, empty-join inputs, and mixed derivation specs.

Copy link
Copy Markdown
Contributor

@amc-corey-cox amc-corey-cox left a comment

Choose a reason for hiding this comment

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

Yep, this looks good to me.

@amc-corey-cox amc-corey-cox merged commit 685b188 into cross-table-lookup Mar 9, 2026
14 checks passed
@amc-corey-cox amc-corey-cox deleted the cross-table-lookup-test-coverage branch March 9, 2026 18:29
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.

3 participants