Merge pull request #572 from spectacles-ci/feature/dangling-content-v2

Report "dangling" content validator errors

spectacles/lookml.py

@@ -172,6 +172,7 @@ def __init__(self, name: str, project_name: str, explores: List[Explore]):
         self.name = name
         self.project_name = project_name
         self.explores = explores
+        self.errors: List[ValidationError] = []
 
     def __repr__(self):
         return f"{self.__class__.__name__}(name={self.name}, explores={self.explores})"
@@ -189,7 +190,9 @@ def __eq__(self, other):
     @property
     def errored(self):
         if self.queried:
-            return any(explore.errored for explore in self.explores)
+            return bool(self.errors) or any(
+                explore.errored for explore in self.explores
+            )
         else:
             return None
 
@@ -236,7 +239,7 @@ def from_json(cls, json_dict):
 
     @property
     def number_of_errors(self):
-        return sum(
+        return len(self.errors) + sum(
             [explore.number_of_errors for explore in self.explores if explore.errored]
         )
 
@@ -322,14 +325,40 @@ def get_explore(self, model: str, name: str) -> Optional[Explore]:
             return model_object.get_explore(name)
 
     def get_results(
-        self, validator: str, fail_fast: Optional[bool] = None
+        self,
+        validator: str,
+        fail_fast: Optional[bool] = None,
+        filters: Optional[List[str]] = None,
     ) -> Dict[str, Any]:
         errors: List[Dict[str, Any]] = []
         successes: List[Dict[str, Any]] = []
         tested = []
 
         for model in self.models:
+            # Add model level content validation errors.
+            # We create an explore "tested" record for those that
+            # aren't in the LookML tree.
+            distinct_explores = set()
+
+            for error in model.errors:
+                if filters is not None and not is_selected(
+                    model.name, error.explore, filters
+                ):
+                    continue
+                distinct_explores.add(error.explore)
+                errors.append(error.to_dict())
+
+            model_tested = [
+                {"model": model.name, "explore": e, "status": "failed"}
+                for e in distinct_explores
+            ]
+            tested.extend(model_tested)
+
             for explore in model.explores:
+                if filters is not None and not is_selected(
+                    model.name, explore.name, filters
+                ):
+                    continue
                 status = "passed"
                 if explore.skipped:
                     status = "skipped"
@@ -401,6 +430,7 @@ def build_project(
     filters: Optional[List[str]] = None,
     include_dimensions: bool = False,
     ignore_hidden_fields: bool = False,
+    include_all_explores: bool = False,
 ) -> Project:
     """Creates an object (tree) representation of a LookML project."""
     if filters is None:
@@ -410,7 +440,7 @@ def build_project(
     fields = ["name", "project_name", "explores"]
     for lookmlmodel in client.get_lookml_models(fields=fields):
         model = Model.from_json(lookmlmodel)
-        if model.project_name == name and model.explores:
+        if model.project_name == name:
             models.append(model)
 
     if not models:
@@ -424,18 +454,26 @@ def build_project(
             ),
         )
 
-    for model in models:
-        model.explores = [
-            explore
-            for explore in model.explores
-            if is_selected(model.name, explore.name, filters)
-        ]
-
-        if include_dimensions:
+    # Prune to selected explores for non-content validators
+    if not include_all_explores:
+        for model in models:
+            model.explores = [
+                explore
+                for explore in model.explores
+                if is_selected(model.name, explore.name, filters)
+            ]
+
+    if include_dimensions:
+        for model in models:
             for explore in model.explores:
                 explore.dimensions = build_dimensions(
                     client, model.name, explore.name, ignore_hidden_fields
                 )
 
-    project = Project(name, [model for model in models if len(model.explores) > 0])
+    # Include empty models when including all explores
+    if include_all_explores:
+        project = Project(name, models)
+    else:
+        project = Project(name, [m for m in models if len(m.explores) > 0])
+
     return project

spectacles/runner.py

@@ -446,15 +446,20 @@ def validate_content(
                 "Building LookML project hierarchy for project "
                 f"'{self.project}' @ {self.branch_manager.ref}"
             )
-            project = build_project(self.client, name=self.project, filters=filters)
+            project = build_project(
+                self.client,
+                name=self.project,
+                filters=filters,
+                include_all_explores=True,
+            )
             explore_count = project.count_explores()
             print_header(
                 f"Validating content based on {explore_count} "
                 f"{'explore' if explore_count == 1 else 'explores'}"
                 + (" [incremental mode] " if incremental else "")
             )
             validator.validate(project)
-            results = project.get_results(validator="content")
+            results = project.get_results(validator="content", filters=filters)
 
         if incremental and (self.branch_manager.branch or self.branch_manager.commit):
             logger.debug("Starting another content validation against the target ref")
@@ -464,10 +469,15 @@ def validate_content(
                     f"'{self.project}' @ {self.branch_manager.ref}"
                 )
                 target_project = build_project(
-                    self.client, name=self.project, filters=filters
+                    self.client,
+                    name=self.project,
+                    filters=filters,
+                    include_all_explores=True,
                 )
                 validator.validate(target_project)
-                target_results = target_project.get_results(validator="content")
+                target_results = target_project.get_results(
+                    validator="content", filters=filters
+                )
 
             return self._incremental_results(base=results, target=target_results)
         else:

spectacles/validators/content.py

@@ -1,7 +1,7 @@
 from typing import List, Optional, Any, Dict
 from spectacles.client import LookerClient
 from spectacles.exceptions import ContentError, SpectaclesException
-from spectacles.lookml import Explore, Project
+from spectacles.lookml import Explore, Project, Model
 from spectacles.logger import GLOBAL_LOGGER as logger
 from spectacles.types import JsonDict
 
@@ -128,11 +128,13 @@ def _get_errors_from_result(
         for error in result["errors"]:
             model_name = error["model_name"]
             explore_name = error["explore_name"]
-            explore: Optional[Explore] = project.get_explore(
-                model=model_name, name=explore_name
-            )
-            # Skip errors that are not associated with selected explores
-            if explore:
+            model: Optional[Model] = project.get_model(model_name)
+            if model:
+                explore: Optional[Explore] = model.get_explore(name=explore_name)
+            else:
+                explore = None
+            # Skip errors that are not associated with selected explores or existing models
+            if explore or model:
                 content_id = result[content_type]["id"]
                 content_error = ContentError(
                     model=model_name,
@@ -154,8 +156,11 @@ def _get_errors_from_result(
                         else None
                     ),
                 )
-                if content_error not in explore.errors:
+                if explore and content_error not in explore.errors:
                     explore.errors.append(content_error)
                     content_errors.append(content_error)
+                elif model and content_error not in model.errors:
+                    model.errors.append(content_error)
+                    content_errors.append(content_error)
 
         return content_errors

tests/test_content_validator.py

@@ -32,10 +32,15 @@ class TestValidatePass:
     @pytest.mark.vcr(match_on=["uri", "method", "raw_body"])
     @pytest.fixture(scope="class")
     def validator_errors(self, validator) -> Iterable[List[ContentError]]:
+        filters = ["eye_exam/users"]
         project = build_project(
-            validator.client, name="eye_exam", filters=["eye_exam/users"]
+            validator.client,
+            name="eye_exam",
+            filters=filters,
+            include_all_explores=True,
         )
-        errors: List[ContentError] = validator.validate(project)
+        validator.validate(project)
+        errors = project.get_results(validator="content", filters=filters)["errors"]
         yield errors
 
     def test_should_not_return_errors(self, validator_errors):
@@ -49,17 +54,22 @@ class TestValidateFail:
     @pytest.mark.vcr(match_on=["uri", "method", "raw_body"])
     @pytest.fixture(scope="class")
     def validator_errors(self, validator) -> Iterable[List[ContentError]]:
+        filters = ["eye_exam/users__fail"]
         project = build_project(
-            validator.client, name="eye_exam", filters=["eye_exam/users__fail"]
+            validator.client,
+            name="eye_exam",
+            filters=filters,
+            include_all_explores=True,
         )
-        errors: List[ContentError] = validator.validate(project)
+        validator.validate(project)
+        errors = project.get_results(validator="content", filters=filters)["errors"]
         yield errors
 
     def test_should_return_errors(self, validator_errors):
         assert len(validator_errors) == 1
 
     def test_personal_folder_content_should_not_be_present(self, validator_errors):
-        titles = [error.metadata["title"] for error in validator_errors]
+        titles = [error["metadata"]["title"] for error in validator_errors]
         # All failing content in personal spaces has been tagged with "[personal]"
         assert "personal" not in titles
 
@@ -71,11 +81,16 @@ class TestValidateFailExcludeFolder:
     @pytest.mark.vcr(match_on=["uri", "method", "raw_body"])
     @pytest.fixture(scope="class")
     def validator_errors(self, validator) -> Iterable[List[ContentError]]:
+        filters = ["eye_exam/users__fail"]
         project = build_project(
-            validator.client, name="eye_exam", filters=["eye_exam/users__fail"]
+            validator.client,
+            name="eye_exam",
+            filters=filters,
+            include_all_explores=True,
         )
         validator.excluded_folders.append(26)
-        errors: List[ContentError] = validator.validate(project)
+        validator.validate(project)
+        errors = project.get_results(validator="content", filters=filters)["errors"]
         yield errors
 
     def test_error_from_excluded_folder_should_be_ignored(self, validator_errors):
@@ -89,11 +104,16 @@ class TestValidateFailIncludeFolder:
     @pytest.mark.vcr(match_on=["uri", "method", "raw_body"])
     @pytest.fixture(scope="class")
     def validator_errors(self, validator) -> Iterable[List[ContentError]]:
+        filters = ["eye_exam/users__fail"]
         project = build_project(
-            validator.client, name="eye_exam", filters=["eye_exam/users__fail"]
+            validator.client,
+            name="eye_exam",
+            filters=filters,
+            include_all_explores=True,
         )
         validator.included_folders.append(26)
-        errors: List[ContentError] = validator.validate(project)
+        validator.validate(project)
+        errors = project.get_results(validator="content", filters=filters)["errors"]
         yield errors
 
     def test_error_from_included_folder_should_be_returned(self, validator_errors):
@@ -107,12 +127,17 @@ class TestValidateFailIncludeExcludeFolder:
     @pytest.mark.vcr(match_on=["uri", "method", "raw_body"])
     @pytest.fixture(scope="class")
     def validator_errors(self, validator) -> Iterable[List[ContentError]]:
+        filters = ["eye_exam/users__fail"]
         project = build_project(
-            validator.client, name="eye_exam", filters=["eye_exam/users__fail"]
+            validator.client,
+            name="eye_exam",
+            filters=filters,
+            include_all_explores=True,
         )
         validator.included_folders.append(26)
         validator.excluded_folders.append(26)
-        errors: List[ContentError] = validator.validate(project)
+        validator.validate(project)
+        errors = project.get_results(validator="content", filters=filters)["errors"]
         yield errors
 
     def test_excluded_folder_should_take_priority_over_included_folder(
@@ -121,6 +146,29 @@ def test_excluded_folder_should_take_priority_over_included_folder(
         assert len(validator_errors) == 0
 
 
+class TestValidateFailOnDeletedExplore:
+    """Test the eye_exam Looker project on master for an explore that has been deleted."""
+
+    @pytest.mark.default_cassette("fixture_validator_fail_deleted_explore.yaml")
+    @pytest.mark.vcr(match_on=["uri", "method", "raw_body"])
+    @pytest.fixture(scope="class")
+    def validator_errors(self, validator) -> Iterable[List[ContentError]]:
+        filters = ["eye_exam/*"]
+        project = build_project(
+            validator.client,
+            name="eye_exam",
+            filters=filters,
+            include_all_explores=True,
+        )
+        validator.validate(project)
+        errors = project.get_results(validator="content", filters=filters)["errors"]
+        yield errors
+
+    def test_error_from_deleted_explore_should_be_present(self, validator_errors):
+        titles = [error["metadata"]["title"] for error in validator_errors]
+        assert "Users [from deleted explore]" in titles
+
+
 def test_non_existing_excluded_folder_should_raise_exception(looker_client):
     with pytest.raises(SpectaclesException):
         ContentValidator(