Ele 5122 cli stop using deprecated tests#2022
Conversation
|
👋 @haritamar |
WalkthroughReplaces a single package integration step with an explicit multi-step E2E pipeline, removes legacy workflow inputs/env vars, adds a full E2E dbt project (configs, packages, models, macros, snapshots, tests, data generator, docker-compose), broadens .gitignore for dbt_packages, and updates workflow references to the new E2E project path. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Trigger as Workflow Trigger
participant Runner as GitHub Runner
participant DBT as dbt CLI
participant E2E as tests/e2e_dbt_project
participant Validator as run_results.json Validator
participant Monitor as EDR Monitor/Report
Trigger->>Runner: start test-warehouse
Runner->>DBT: (wd: $E2E_DBT_PROJECT_DIR) dbt deps
Runner->>DBT: (wd: $E2E_DBT_PROJECT_DIR) dbt seed
DBT-->>Runner: seeds loaded
Runner->>DBT: (wd: $E2E_DBT_PROJECT_DIR) dbt run
DBT-->>Runner: writes run_results.json
Runner->>Validator: validate run_results.json (only designated model may fail)
alt validation OK
Runner->>DBT: (wd: $E2E_DBT_PROJECT_DIR) dbt test
DBT-->>Runner: test results
Runner->>Monitor: run edr monitor/report and collect artifacts
else validation FAIL
Runner-->>Trigger: fail workflow (unexpected failures)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
tests/e2e_dbt_project/models/test_alerts_union.sql (1)
12-18: Avoid SELECT * across UNIONs; enumerate columns to prevent drift.Explicitly list columns for dbt, anomalies, and schema_changes to avoid brittle breakages if upstream models change column order.
tests/e2e_dbt_project/models/backfill_days_column_anomalies.sql (1)
15-23: Consistency: select explicit columns in the final projection.Keep the outer query selecting the explicit column list instead of
select *to guard against upstream changes.tests/e2e_dbt_project/models/no_timestamp_anomalies.sql (1)
15-30: Nit: remove leading space beforefinalCTE and avoidselect *in outer query.Minor readability/style; and explicitly list columns in the outer select to prevent schema drift issues.
tests/e2e_dbt_project/models/any_type_column_anomalies.sql (1)
15-30: Mirror the projection in the outer query; avoidselect *.Explicit outer select prevents brittle coupling to CTE internals.
tests/e2e_dbt_project/generate_data.py (2)
38-42: CSV writer: add newline='' and encoding to avoid extra blank lines and ensure UTF-8.On some platforms CSVs will contain blank lines without
newline=''. Also setencoding="utf-8"explicitly.- with open(csv_path, "w") as csv_file: + with open(csv_path, "w", newline="", encoding="utf-8") as csv_file: writer = csv.DictWriter(csv_file, fieldnames=header) writer.writeheader() writer.writerows(rows)
404-421: Silence lints: unused parameters in nested row builders.
row_indexandrows_countaren’t used in these closures; rename to_/__to satisfy linters.-def get_training_row(date, row_index, rows_count): +def get_training_row(date, _row_index, _rows_count): @@ -def get_validation_row(date, row_index, rows_count): +def get_validation_row(date, _row_index, _rows_count):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (16)
tests/e2e_dbt_project/data/training/any_type_column_anomalies_training.csvis excluded by!**/*.csvtests/e2e_dbt_project/data/training/backfill_days_column_anomalies_training.csvis excluded by!**/*.csvtests/e2e_dbt_project/data/training/dimension_anomalies_training.csvis excluded by!**/*.csvtests/e2e_dbt_project/data/training/groups_training.csvis excluded by!**/*.csvtests/e2e_dbt_project/data/training/numeric_column_anomalies_training.csvis excluded by!**/*.csvtests/e2e_dbt_project/data/training/stats_players_training.csvis excluded by!**/*.csvtests/e2e_dbt_project/data/training/stats_team_training.csvis excluded by!**/*.csvtests/e2e_dbt_project/data/training/string_column_anomalies_training.csvis excluded by!**/*.csvtests/e2e_dbt_project/data/validation/any_type_column_anomalies_validation.csvis excluded by!**/*.csvtests/e2e_dbt_project/data/validation/backfill_days_column_anomalies_validation.csvis excluded by!**/*.csvtests/e2e_dbt_project/data/validation/dimension_anomalies_validation.csvis excluded by!**/*.csvtests/e2e_dbt_project/data/validation/groups_validation.csvis excluded by!**/*.csvtests/e2e_dbt_project/data/validation/numeric_column_anomalies_validation.csvis excluded by!**/*.csvtests/e2e_dbt_project/data/validation/stats_players_validation.csvis excluded by!**/*.csvtests/e2e_dbt_project/data/validation/stats_team_validation.csvis excluded by!**/*.csvtests/e2e_dbt_project/data/validation/string_column_anomalies_validation.csvis excluded by!**/*.csv
📒 Files selected for processing (38)
.github/workflows/test-warehouse.yml(1 hunks).gitignore(1 hunks)tests/e2e_dbt_project/README.md(1 hunks)tests/e2e_dbt_project/dbt_project.yml(1 hunks)tests/e2e_dbt_project/debug.sh(1 hunks)tests/e2e_dbt_project/generate_data.py(1 hunks)tests/e2e_dbt_project/macros/generic_tests/generic_test_on_column.sql(1 hunks)tests/e2e_dbt_project/macros/generic_tests/generic_test_on_model.sql(1 hunks)tests/e2e_dbt_project/macros/system/dbg.sql(1 hunks)tests/e2e_dbt_project/macros/system/generate_schema_name.sql(1 hunks)tests/e2e_dbt_project/macros/system/materializations.sql(1 hunks)tests/e2e_dbt_project/models/any_type_column_anomalies.sql(1 hunks)tests/e2e_dbt_project/models/backfill_days_column_anomalies.sql(1 hunks)tests/e2e_dbt_project/models/config_levels_project.sql(1 hunks)tests/e2e_dbt_project/models/config_levels_test_and_model.sql(1 hunks)tests/e2e_dbt_project/models/copy_numeric_column_anomalies.sql(1 hunks)tests/e2e_dbt_project/models/dimension_anomalies.sql(1 hunks)tests/e2e_dbt_project/models/ephemeral_model.sql(1 hunks)tests/e2e_dbt_project/models/error_model.sql(1 hunks)tests/e2e_dbt_project/models/groups.sql(1 hunks)tests/e2e_dbt_project/models/nested/models/tree/nested.sql(1 hunks)tests/e2e_dbt_project/models/no_timestamp_anomalies.sql(1 hunks)tests/e2e_dbt_project/models/non_dbt_model.sql(1 hunks)tests/e2e_dbt_project/models/numeric_column_anomalies.sql(1 hunks)tests/e2e_dbt_project/models/one.sql(1 hunks)tests/e2e_dbt_project/models/schema.yml(1 hunks)tests/e2e_dbt_project/models/stats_players.sql(1 hunks)tests/e2e_dbt_project/models/stats_team.sql(1 hunks)tests/e2e_dbt_project/models/string_column_anomalies.sql(1 hunks)tests/e2e_dbt_project/models/test_alerts_union.sql(1 hunks)tests/e2e_dbt_project/models/users_per_day_weekly_seasonal.sql(1 hunks)tests/e2e_dbt_project/models/users_per_hour_daily_seasonal.sql(1 hunks)tests/e2e_dbt_project/packages.yml(1 hunks)tests/e2e_dbt_project/snapshots/failed_snapshot.sql(1 hunks)tests/e2e_dbt_project/tests/singular_test_with_no_ref.sql(1 hunks)tests/e2e_dbt_project/tests/singular_test_with_one_ref.sql(1 hunks)tests/e2e_dbt_project/tests/singular_test_with_source_ref.sql(1 hunks)tests/e2e_dbt_project/tests/singular_test_with_two_refs.sql(1 hunks)
🧰 Additional context used
🪛 Ruff (0.13.3)
tests/e2e_dbt_project/generate_data.py
59-59: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
59-59: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
62-62: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
62-62: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
64-64: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
67-67: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
69-69: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
70-70: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
78-78: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
78-78: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
81-81: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
81-81: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
84-84: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
84-84: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
88-88: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
90-90: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
91-91: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
131-131: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
132-132: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
135-135: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
137-137: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
138-138: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
139-139: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
140-140: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
141-141: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
142-142: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
150-150: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
151-151: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
154-154: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
156-156: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
157-157: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
158-158: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
159-159: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
160-160: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
161-161: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
206-206: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
208-208: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
209-209: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
212-212: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
214-214: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
215-215: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
218-218: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
220-220: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
221-221: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
226-226: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
236-236: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
238-238: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
239-239: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
242-242: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
244-244: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
245-245: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
248-248: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
250-250: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
251-251: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
256-256: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
303-303: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
311-311: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
344-344: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
363-363: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
404-404: Unused function argument: row_index
(ARG001)
404-404: Unused function argument: rows_count
(ARG001)
409-409: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
409-409: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
413-413: Unused function argument: row_index
(ARG001)
413-413: Unused function argument: rows_count
(ARG001)
418-418: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
418-418: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test / test
- GitHub Check: code-quality
🔇 Additional comments (5)
tests/e2e_dbt_project/models/stats_team.sql (1)
1-1: LGTM for passthrough model dependency.
This passthrough keepsstats_teamaligned withstats_team_validationso downstream tests see identical columns—looks good.tests/e2e_dbt_project/models/stats_players.sql (1)
1-1: LGTM for passthrough model dependency.
Mirrors the validation model cleanly; no issues spotted.tests/e2e_dbt_project/models/groups.sql (1)
1-1: Pass-through model looks good.Matches the existing pattern of proxy models and correctly references
groups_validation.tests/e2e_dbt_project/macros/system/materializations.sql (1)
1-7: Confirm overriding dbt’s built-intestmaterialization is intentional and compatible.dbt defines
materialization testby default. Overriding it can affect all tests. Ensureelementary.materialization_test_default/snowflakereturn a valid materialization result and align with the dbt-core version under test.tests/e2e_dbt_project/generate_data.py (1)
53-93: Ruff S311 warnings are expected here.Use of
randomis fine for non-crypto test data generation. No action needed.If you want to suppress S311 in CI for this file, we can add
# noqa: S311selectively or configure Ruff to ignore S311 under tests/.Also applies to: 126-163, 199-259, 297-331, 334-377, 380-396, 398-401, 403-447
2baebad to
8ce421e
Compare
|
@haritamar I see you didn't copy all the macros from the dbt pacakge (the |
null
Summary by CodeRabbit
Tests
Documentation
Chores