Prevent duplicated rule exception when importing rules and filters (#87)

Fixes #85. ### Implementation - Skip adding imported rules and rule filters to rule registry. - Split `tests/rules/example.py` into `tests/rules/rules.py` and `tests/rules/rule_filters.py`. - Test `Duplicated` exception is not raised by adding an imported `skip_schemaX` rule filter. ### Note Both rules and rule filters can be imported now, but the existing tests cover only rule filters. --------- Co-authored-by: Matthieu Caneill <matthieucan@users.noreply.github.com>
PicnicSupermarket · Dec 19, 2024 · 2c47aa9 · 2c47aa9
1 parent c8a7d27
commit 2c47aa9
Show file tree

Hide file tree

Showing 9 changed files with 53 additions and 37 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@ and this project adheres to
 - Documenting support for python 3.13. (#86)
 - Only show failing rules per default in `HumanReadableFormatter`. Also added
   `--show` parameter in the CLI to change this behavior. (#77)
+- Ignore imported rules and filters when building the rule registry. (#87)
 
 ## [0.8.0] - 2024-11-12
 

diff --git a/src/dbt_score/rule_registry.py b/src/dbt_score/rule_registry.py
@@ -62,6 +62,9 @@ def _load(self, namespace_name: str) -> None:
             module = importlib.import_module(module_name)
             for obj_name in dir(module):
                 obj = module.__dict__[obj_name]
+                # Skip adding objects imported from other modules
+                if type(obj) is type and module.__name__ != obj.__module__:
+                    continue
                 if type(obj) is type and issubclass(obj, Rule) and obj is not Rule:
                     self._add_rule(obj)
                 if (

diff --git a/tests/resources/pyproject.toml b/tests/resources/pyproject.toml
@@ -23,6 +23,6 @@ severity=4
 [tool.dbt-score.rules."tests.conftest.rule_with_config"]
 model_name="model2"
 
-[tool.dbt-score.rules."tests.rules.example.rule_test_example"]
+[tool.dbt-score.rules."tests.rules.rules.rule_test_example"]
 severity=4
-rule_filter_names=["tests.rules.example.skip_model1"]
+rule_filter_names=["tests.rules.rule_filters.skip_model1"]
diff --git a/tests/rules/example.py b/tests/rules/example.py
diff --git a/tests/rules/rule_filters.py b/tests/rules/rule_filters.py
@@ -0,0 +1,15 @@
+"""Example rule filters."""
+
+from dbt_score import Model, rule_filter
+
+
+@rule_filter
+def skip_model1(model: Model) -> bool:
+    """An example filter."""
+    return model.name != "model1"
+
+
+@rule_filter
+def skip_schemaX(model: Model) -> bool:
+    """An example filter."""
+    return model.schema != "X"
diff --git a/tests/rules/rules.py b/tests/rules/rules.py
@@ -0,0 +1,10 @@
+"""Example rules."""
+
+from dbt_score import Model, RuleViolation, rule
+
+from tests.rules.rule_filters import skip_schemaX
+
+
+@rule(rule_filters={skip_schemaX()})
+def rule_test_example(model: Model) -> RuleViolation | None:
+    """An example rule."""
diff --git a/tests/test_config.py b/tests/test_config.py
@@ -15,7 +15,7 @@ def test_load_valid_toml_file(valid_config_path):
     assert config.disabled_rules == ["foo.foo", "tests.bar"]
     assert config.rules_config["foo.bar"].severity == Severity.CRITICAL
     assert (
-        config.rules_config["tests.rules.example.rule_test_example"].severity
+        config.rules_config["tests.rules.rules.rule_test_example"].severity
         == Severity.CRITICAL
     )
     assert config.badge_config.third.threshold == 6.5
@@ -28,8 +28,8 @@ def test_load_valid_toml_file(valid_config_path):
     assert config.fail_project_under == 7.5
     assert config.fail_any_item_under == 6.9
     assert config.rules_config[
-        "tests.rules.example.rule_test_example"
-    ].rule_filter_names == ["tests.rules.example.skip_model1"]
+        "tests.rules.rules.rule_test_example"
+    ].rule_filter_names == ["tests.rules.rule_filters.skip_model1"]
 
 
 def test_load_invalid_toml_file(caplog, invalid_config_path):

diff --git a/tests/test_rule_catalog.py b/tests/test_rule_catalog.py
@@ -10,10 +10,10 @@ def test_rule_catalog_terminal(capsys, default_config):
     stdout = capsys.readouterr().out
     assert (
         stdout
-        == """\x1B[1mtests.rules.example.rule_test_example\x1B[0m:
+        == """\x1B[1mtests.rules.nested.example.rule_test_nested_example\x1B[0m:
     An example rule.
 
-\x1B[1mtests.rules.nested.example.rule_test_nested_example\x1B[0m:
+\x1B[1mtests.rules.rules.rule_test_example\x1B[0m:
     An example rule.
 
 """
@@ -29,41 +29,41 @@ def test_rule_catalog_markdown(capsys, default_config):
         stdout
         == """# Doc for tests.rules
 
-## `rule_test_example`
+## `rule_test_nested_example`
 
 An example rule.
 
 ??? quote  "Source code"
     ```python
-    @rule()
-    def rule_test_example(model: Model) -> RuleViolation | None:
+    @rule
+    def rule_test_nested_example(model: Model) -> RuleViolation | None:
         \"""An example rule.\"""
 
     ```
 
 ### Default configuration
 
 ```toml title="pyproject.toml"
-[tool.dbt-score.rules."tests.rules.example.rule_test_example"]
+[tool.dbt-score.rules."tests.rules.nested.example.rule_test_nested_example"]
 severity = 2
 ```
 
-## `rule_test_nested_example`
+## `rule_test_example`
 
 An example rule.
 
 ??? quote  "Source code"
     ```python
-    @rule
-    def rule_test_nested_example(model: Model) -> RuleViolation | None:
+    @rule(rule_filters={skip_schemaX()})
+    def rule_test_example(model: Model) -> RuleViolation | None:
         \"""An example rule.\"""
 
     ```
 
 ### Default configuration
 
 ```toml title="pyproject.toml"
-[tool.dbt-score.rules."tests.rules.nested.example.rule_test_nested_example"]
+[tool.dbt-score.rules."tests.rules.rules.rule_test_example"]
 severity = 2
 ```
 

diff --git a/tests/test_rule_registry.py b/tests/test_rule_registry.py
@@ -12,10 +12,13 @@ def test_rule_registry_discovery(default_config):
     r = RuleRegistry(default_config)
     r._load("tests.rules")
     assert sorted(r._rules.keys()) == [
-        "tests.rules.example.rule_test_example",
         "tests.rules.nested.example.rule_test_nested_example",
+        "tests.rules.rules.rule_test_example",
+    ]
+    assert list(r._rule_filters.keys()) == [
+        "tests.rules.rule_filters.skip_model1",
+        "tests.rules.rule_filters.skip_schemaX",
     ]
-    assert list(r._rule_filters.keys()) == ["tests.rules.example.skip_model1"]
 
 
 def test_disabled_rule_registry_discovery():
@@ -25,7 +28,7 @@ def test_disabled_rule_registry_discovery():
     r = RuleRegistry(config)
     r._load("tests.rules")
     assert sorted(r._rules.keys()) == [
-        "tests.rules.example.rule_test_example",
+        "tests.rules.rules.rule_test_example",
     ]
 
 
@@ -35,9 +38,7 @@ def test_configured_rule_registry_discovery(valid_config_path):
     config._load_toml_file(str(valid_config_path))
     r = RuleRegistry(config)
     r._load("tests.rules")
-    assert (
-        r.rules["tests.rules.example.rule_test_example"].severity == Severity.CRITICAL
-    )
+    assert r.rules["tests.rules.rules.rule_test_example"].severity == Severity.CRITICAL
 
 
 def test_rule_registry_no_duplicates(default_config):
@@ -63,5 +64,5 @@ def test_rule_registry_rule_filters(valid_config_path, model1, model2):
     r._load("tests.rules")
     r._load_filters_into_rules()
 
-    assert not r.rules["tests.rules.example.rule_test_example"].should_evaluate(model1)
-    assert r.rules["tests.rules.example.rule_test_example"].should_evaluate(model2)
+    assert not r.rules["tests.rules.rules.rule_test_example"].should_evaluate(model1)
+    assert r.rules["tests.rules.rules.rule_test_example"].should_evaluate(model2)