Skip to content

refactor: move universal tests to sdk/python/tests/universal#6077

Open
shenganzhang wants to merge 4 commits intofeast-dev:masterfrom
shenganzhang:fix/test-restructure-3-6051
Open

refactor: move universal tests to sdk/python/tests/universal#6077
shenganzhang wants to merge 4 commits intofeast-dev:masterfrom
shenganzhang:fix/test-restructure-3-6051

Conversation

@shenganzhang
Copy link

@shenganzhang shenganzhang commented Mar 8, 2026

What this PR does / why we need it:

This PR restructures cross-backend tests by moving them from sdk/python/tests/integration/* into dedicated sdk/python/tests/universal/* directories, consistent with the test taxonomy proposed in issue #6051.

It also relocates non-universal outliers to their intended locations:

  • test_remote_online_store.py -> integration/auth/
  • test_hybrid_online_store.py -> unit/

In addition, this PR updates references to moved test files in:

  • Makefile
  • sdk/python/tests/README.md
  • docs/how-to-guides/adding-or-reusing-tests.md

Which issue(s) this PR fixes:

Fixes #6051


Open with Devin

@shenganzhang shenganzhang requested a review from a team as a code owner March 8, 2026 07:46
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

🐛 1 issue in files not directly in the diff

🐛 Moved test_universal_registry.py loses access to session-scoped fixtures from integration/conftest.py (sdk/python/tests/universal/registration/test_universal_registry.py:101)

The file test_universal_registry.py was moved from sdk/python/tests/integration/registration/ to sdk/python/tests/universal/registration/, but it depends on fixtures (minio_server, postgres_server, mysql_server) defined in sdk/python/tests/integration/conftest.py:135-162. Pytest conftest discovery only loads conftest files in the directory hierarchy from rootdir down to the test file's directory. Since universal/ is a sibling of integration/, not a child, the fixtures in integration/conftest.py will not be discovered for tests under universal/. There is no conftest.py in the universal/ directory tree, and the parent sdk/python/tests/conftest.py does not define these fixtures. This will cause fixture 'minio_server' not found (and similar) errors for at least 8 test functions that use these fixtures (e.g., test_universal_registry.py:101 uses minio_server, test_universal_registry.py:163 uses postgres_server, test_universal_registry.py:230 uses mysql_server).

View 4 additional findings in Devin Review.

Open in Devin Review

Restructure cross-backend Python tests into new universal directories, and relocate auth/unit outliers to align with the test taxonomy in issue feast-dev#6051 while updating docs and Makefile paths.

Made-with: Cursor
@shenganzhang shenganzhang force-pushed the fix/test-restructure-3-6051 branch from 2d52eb1 to 1cb92df Compare March 14, 2026 22:55
devin-ai-integration[bot]

This comment was marked as resolved.

Point integration test targets to ignore sdk/python/tests/universal/registration after the test restructure so registration suites remain excluded as intended.

Signed-off-by: zshengan <zshengan@uber.com>
Made-with: Cursor
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 3 new potential issues.

View 7 additional findings in Devin Review.

Open in Devin Review

python -m pytest -x --integration \
sdk/python/tests/integration/offline_store/test_feature_logging.py \
sdk/python/tests/universal/offline_store/test_feature_logging.py \
--ignore=sdk/python/tests/integration/offline_store/test_validation.py \
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 Stale --ignore path references non-existent file test_validation.py

In the test-python-universal-cassandra Makefile target, the --ignore path at line 478 still references sdk/python/tests/integration/offline_store/test_validation.py. This file was renamed to test_dqm_validation.py in a previous commit and then moved to sdk/python/tests/universal/offline_store/test_dqm_validation.py in this PR. The --ignore flag silently does nothing when the path doesn't exist, so the intended test exclusion is not applied. Since this PR updated other paths in this same target (line 477), the stale reference should have been updated as well.

Suggested change
--ignore=sdk/python/tests/integration/offline_store/test_validation.py \
--ignore=sdk/python/tests/universal/offline_store/test_dqm_validation.py \
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

zshengan added 2 commits March 14, 2026 16:39
Add missing __init__.py under sdk/python/tests/integration/auth after moving test_remote_online_store.py to keep package structure consistent.

Signed-off-by: zshengan <zshengan@uber.com>
Made-with: Cursor
Add __init__.py files to moved universal test subdirectories so pytest module imports and package discovery remain consistent.

Signed-off-by: zshengan <zshengan@uber.com>
Made-with: Cursor
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.

[Test restructure 3] Move universal tests to universal directory

1 participant