-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: Add integration tests for dbt import #5899
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
YassinNouh21
wants to merge
25
commits into
feast-dev:master
Choose a base branch
from
YassinNouh21:copilot/add-integration-tests-for-dbt-import
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,637
−3
Open
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
c3f2e0f
Initial plan
Copilot 9bee2c7
Add dbt integration tests and test dbt project
Copilot 8ba2205
Add manifest.json and GitHub Actions workflow for dbt tests
Copilot 3432eb5
Add documentation and README files for dbt integration tests
Copilot 621cefb
Use Makefile for dependency installation in dbt workflow
Copilot e25d08f
Simplify profiles.yml to only use DuckDB for testing
Copilot ba38da0
Add dbt commands and Feast CLI integration testing with SQLite
Copilot 586f825
Fix test failures by removing pre-committed manifest and letting dbt …
Copilot 6354024
Fix dbt integration tests and add comprehensive test coverage
YassinNouh21 7f023b1
Remove unnecessary README documentation files
YassinNouh21 37224b0
Fix CI: remove redundant dbt test command and unused imports
YassinNouh21 e45bcb6
Fix feast CLI: remove incorrect -c flag
YassinNouh21 1da03ea
Fix dbt integration tests to work with dynamic dbt-generated values
YassinNouh21 9818f0c
Fix dbt schema.yml to use config block for tags
YassinNouh21 cb09148
Fix lint errors in dbt integration tests
YassinNouh21 932c559
Apply ruff formatting to dbt integration tests
YassinNouh21 125bb29
Skip dbt integration tests when manifest.json doesn't exist
YassinNouh21 3edce4f
Merge branch 'master' into copilot/add-integration-tests-for-dbt-import
YassinNouh21 c302366
Address PR review comments
YassinNouh21 6954f13
Merge branch 'master' into copilot/add-integration-tests-for-dbt-import
franciscojavierarceo e31a11a
Merge branch 'master' into copilot/add-integration-tests-for-dbt-import
YassinNouh21 7e26e76
Fix test API mismatches and add comprehensive integration test coverage
YassinNouh21 931504e
Fix import sorting lint errors in dbt integration tests
YassinNouh21 d05b131
Fix ruff formatting in dbt integration tests
YassinNouh21 4318134
Fix entity column exclusion in codegen and type widening in mapper
YassinNouh21 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| name: dbt-integration-tests | ||
|
|
||
| # Run dbt integration tests on PRs | ||
| on: | ||
| pull_request: | ||
| paths: | ||
| - 'sdk/python/feast/dbt/**' | ||
| - 'sdk/python/tests/integration/dbt/**' | ||
| - 'sdk/python/tests/unit/dbt/**' | ||
| - '.github/workflows/dbt-integration-tests.yml' | ||
|
|
||
| jobs: | ||
| dbt-integration-test: | ||
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| python-version: ["3.11", "3.12"] | ||
| env: | ||
| PYTHON: ${{ matrix.python-version }} | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Setup Python | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: ${{ matrix.python-version }} | ||
| architecture: x64 | ||
|
|
||
| - name: Install the latest version of uv | ||
| uses: astral-sh/setup-uv@v5 | ||
| with: | ||
| enable-cache: true | ||
|
|
||
| - name: Install dependencies | ||
| run: make install-python-dependencies-ci | ||
|
|
||
| - name: Install dbt and dbt-duckdb | ||
| run: | | ||
| uv pip install --system dbt-core dbt-duckdb | ||
|
|
||
| - name: Run dbt integration tests | ||
| run: make test-python-integration-dbt | ||
|
|
||
| - name: Minimize uv cache | ||
| run: uv cache prune --ci |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| # Integration tests for dbt import functionality |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| """ | ||
| Conftest for dbt integration tests. | ||
|
|
||
| This is a standalone conftest that doesn't depend on the main Feast test infrastructure. | ||
| """ | ||
|
|
||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
|
|
||
| # This conftest is minimal and doesn't import the main feast conftest | ||
| # to avoid complex dependency chains for dbt-specific tests | ||
|
|
||
| # Path to the test dbt project manifest | ||
| TEST_DBT_PROJECT_DIR = Path(__file__).parent / "test_dbt_project" | ||
| TEST_MANIFEST_PATH = TEST_DBT_PROJECT_DIR / "target" / "manifest.json" | ||
|
|
||
|
|
||
| def pytest_collection_modifyitems(config, items): # noqa: ARG001 | ||
| """ | ||
| Skip dbt integration tests if manifest.json doesn't exist. | ||
|
|
||
| These tests require running 'dbt build' first to generate the manifest. | ||
| The dbt-integration-test workflow handles this, but regular unit test | ||
| runs don't, so we skip them to avoid failures. | ||
| """ | ||
| if not TEST_MANIFEST_PATH.exists(): | ||
| skip_marker = pytest.mark.skip( | ||
| reason="dbt manifest.json not found - run 'dbt build' first or use dbt-integration-test workflow" | ||
| ) | ||
| for item in items: | ||
| item.add_marker(skip_marker) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| [pytest] | ||
| # Prevent loading parent conftest.py files which may import heavy dependencies | ||
| # like ray that are not needed for dbt integration tests | ||
| norecursedirs = .. | ||
| asyncio_mode = auto | ||
|
|
||
| # Test markers | ||
| markers = | ||
| dbt: dbt integration tests |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Makefile
echowith\nescape sequences relies on shell-specific behavior, fails on bashThe
echocommand on line 200 uses\nto produce newlines in the generatedfeature_store.yaml, but POSIXechobehavior with escape sequences is implementation-defined. On systems where/bin/shisbash(or when Make'sSHELLis set to bash),echodoes NOT interpret\n, producing a single-line file with literal\ntext — resulting in invalid YAML that causes the subsequentfeast dbt importcommand to fail.Root Cause and Impact
The command:
On Ubuntu CI,
/bin/shisdash, which interprets\ninecho— so this works in CI. However, on macOS or systems wherebashis the default shell,echooutputs literal\n, producing:This is a single line of invalid YAML, causing
feast dbt importon the next recipe line to fail with a YAML parsing error. Usingprintfinstead ofechois the portable fix, sinceprintfalways interprets escape sequences.Impact:
make test-python-integration-dbtfails on developer machines running macOS or any system where Make's shell is bash.Was this helpful? React with 👍 or 👎 to provide feedback.