refactor: move universal tests to sdk/python/tests/universal#6077
refactor: move universal tests to sdk/python/tests/universal#6077shenganzhang wants to merge 6 commits into
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
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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
franciscojavierarceo
left a comment
There was a problem hiding this comment.
i think Devin caught a couple of tests that need to be fixed, otherwise lgtm
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Update Makefile and pixi task test paths to use the moved universal registration and offline store directories so excludes and targeted runs still execute the intended suites. Signed-off-by: zshengan <zshengan@uber.com> Made-with: Cursor
thanks, that is resolved @franciscojavierarceo |
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