fix: restore missing test macros in e2e dbt project#2138
Conversation
… e2e project - Add test_config_levels generic test macro that was removed from dbt-data-reliability in PR #947 but still referenced by the e2e project - Fix 'uniques' -> 'unique' (standard dbt generic test) on error_model Co-Authored-By: Itamar Hartstein <haritamar@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
👋 @devin-ai-integration[bot] |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request adds a new dbt test called Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| - name: "missing_column" | ||
| tests: | ||
| - uniques: | ||
| - unique: |
There was a problem hiding this comment.
I believe this typo may have been intentional originally, to have an error case for tests
Double check me though
There was a problem hiding this comment.
You're right — uniques is intentional. The error_model with missing_column is tagged error_test, and the non-existent uniques test creates a compilation error case that gets validated by the edr monitoring steps. Reverted in f2d740b.
The 'uniques' test (non-existent) on error_model.missing_column was intentionally creating a compilation error case, tagged as error_test. Co-Authored-By: Itamar Hartstein <haritamar@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/e2e_dbt_project/macros/generic_tests/test_config_levels.sql`:
- Around line 6-17: The code calls
elementary.get_anomalies_test_configuration(...) with mandatory_params which is
undefined; fix by adding mandatory_params as a parameter to the surrounding
macro's signature (or define it locally before the call) and provide a sensible
default (e.g., empty dict) so the call uses a defined value; update the macro
signature where it is declared and ensure the variable name mandatory_params is
passed through to the elementary.get_anomalies_test_configuration(...) call so
that the function receives a defined argument.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
tests/e2e_dbt_project/macros/generic_tests/test_config_levels.sqltests/e2e_dbt_project/models/schema.yml
| {% set configuration_dict, metric_properties_dict = | ||
| elementary.get_anomalies_test_configuration(model_relation, | ||
| mandatory_params, | ||
| timestamp_column, | ||
| where_expression, | ||
| anomaly_sensitivity, | ||
| anomaly_direction, | ||
| min_training_set_size, | ||
| time_bucket, | ||
| days_back, | ||
| backfill_days, | ||
| seasonality) %} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find current get_anomalies_test_configuration definitions/call sites =="
rg -n -C3 'get_anomalies_test_configuration\s*\(' -g '*.sql' | head -60
echo
echo "== Find where mandatory_params is defined or set =="
rg -n -C3 '\bmandatory_params\b' -g '*.sql' | head -80
echo
echo "== Inspect the full test_config_levels.sql file =="
cat -n tests/e2e_dbt_project/macros/generic_tests/test_config_levels.sqlRepository: elementary-data/elementary
Length of output: 6115
mandatory_params is undefined and will cause a runtime error.
The macro parameter list does not include mandatory_params (line 1), yet it is passed to elementary.get_anomalies_test_configuration() at line 8. This variable is not defined locally or available in the macro scope, which will cause a compilation/runtime failure when the test executes. Either add mandatory_params to the macro parameters, define it locally before use, or remove it from the function call if it's no longer needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/e2e_dbt_project/macros/generic_tests/test_config_levels.sql` around
lines 6 - 17, The code calls elementary.get_anomalies_test_configuration(...)
with mandatory_params which is undefined; fix by adding mandatory_params as a
parameter to the surrounding macro's signature (or define it locally before the
call) and provide a sensible default (e.g., empty dict) so the call uses a
defined value; update the macro signature where it is declared and ensure the
variable name mandatory_params is passed through to the
elementary.get_anomalies_test_configuration(...) call so that the function
receives a defined argument.
There was a problem hiding this comment.
Good catch — mandatory_params was available in the original calling context (e.g., test_table_anomalies has mandatory_params=none as a parameter). Since test_config_levels doesn't need mandatory param validation, I've defined it locally as none before the call. Fixed in 9f68a6f.
There was a problem hiding this comment.
Skipped: comment is from another GitHub bot.
The macro calls elementary.get_anomalies_test_configuration() with mandatory_params but it was never defined in scope. In the original dbt-data-reliability code, this was a parameter on the calling test macros. Since test_config_levels doesn't need mandatory param validation, define it locally as none. Co-Authored-By: Itamar Hartstein <haritamar@gmail.com>
1 participant
Summary
Fixes two
dbt testcompilation errors in the e2e project that were introduced when dbt-data-reliability#947 removed the deprecated integration tests directory. The e2e project'sschema.ymlstill references macros/tests that lived there.Changes:
test_config_levelsgeneric test macro — copied verbatim from the removedintegration_tests/deprecated_tests/macros/e2e_tests/test_config_levels.sql. This macro validates that elementary's anomaly detection config resolution (project/model/test level) works correctly. It's referenced by 5 tests inschema.yml.uniques→unique— typo inerror_model.missing_columntest.uniquesis not a valid dbt generic test; the built-in isunique.Review & Testing Checklist for Human
test_config_levelsmacro is still compatible with the currentelementary.get_anomalies_test_configuration()API — it was copied from code removed ~recently but the macro signature may have driftedmandatory_params(referenced on line 8 of the macro but not passed as an argument) resolves correctly at runtime — it appears to be an implicit dbt context variable, not a bug, but worth confirmingunique(notuniques) is the intended test forerror_model.missing_column— this test is taggederror_testso it's expected to fail at runtime regardlessNotes
Summary by CodeRabbit