Skip to content

Conversation

@nssalian
Copy link
Contributor

@nssalian nssalian commented Jan 13, 2026

Closes #2870

Rationale for this change

Eliminates code duplication between test_base.py and test_sql.py by consolidating shared catalog behavior tests into a single parameterized test suite. This ensures consistent behavior testing across both catalog implementations and makes it easier to add new catalog implementations with shared test coverage.

Changes

  • New tests/catalog/test_catalog_behaviors.py with parameterized tests covering both catalogs
  • Added catalog fixtures to tests/conftest.py
  • Reduced test_base.py to InMemoryCatalog-specific tests
  • Reduced test_sql.py to SqlCatalog-specific tests
  • Fixed return type annotation for test_partition_spec fixture in tests/conftest.py
  • Removed an import from tests/table/test_upsert.py

Are these changes tested?

make test ran successfully locally

Are there any user-facing changes?

No

@nssalian nssalian marked this pull request as draft January 13, 2026 04:38
@nssalian nssalian marked this pull request as ready for review January 13, 2026 05:09
Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

Nice one @nssalian Thanks for consolidating this, much cleaner!

@Fokko Fokko merged commit b3a8027 into apache:main Jan 13, 2026
10 checks passed
@nssalian nssalian deleted the test-consolidation branch January 13, 2026 22:01
@jayceslesar jayceslesar mentioned this pull request Jan 14, 2026
3 tasks
kevinjqliu pushed a commit that referenced this pull request Jan 14, 2026
Closes #2911 

# Rationale for this change
PR #2906 added imports to `tests/conftest.py` that require
`pytest-lazy-fixture` and `sqlalchemy`. The nightly build's wheel test
uses a minimal environment that doesn't include these dependencies,
causing `tests/avro/test_decoder.py` to fail when loading `conftest.py`.

## Are these changes tested?
Yes. Reproduced locally by creating a minimal venv with only the
original test dependencies, reproduced the failure, then verified the
fix resolves it.

## Are there any user-facing changes?
No
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.

Consolidate duplicated catalog behavior tests into parameterized test suite

2 participants