refactor: move universal tests to sdk/python/tests/universal#6077
refactor: move universal tests to sdk/python/tests/universal#6077shenganzhang wants to merge 4 commits intofeast-dev:masterfrom
Conversation
There was a problem hiding this comment.
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.
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
2d52eb1 to
1cb92df
Compare
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
| 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 \ |
There was a problem hiding this comment.
🟡 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.
| --ignore=sdk/python/tests/integration/offline_store/test_validation.py \ | |
| --ignore=sdk/python/tests/universal/offline_store/test_dqm_validation.py \ |
Was this helpful? React with 👍 or 👎 to provide feedback.
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
What this PR does / why we need it:
This PR restructures cross-backend tests by moving them from
sdk/python/tests/integration/*into dedicatedsdk/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:
Makefilesdk/python/tests/README.mddocs/how-to-guides/adding-or-reusing-tests.mdWhich issue(s) this PR fixes:
Fixes #6051