diff --git a/spectacles/lookml.py b/spectacles/lookml.py index 88e73fed..5cbd11dc 100644 --- a/spectacles/lookml.py +++ b/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 diff --git a/spectacles/runner.py b/spectacles/runner.py index 4ea30dfd..8a61be14 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -446,7 +446,12 @@ 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} " @@ -454,7 +459,7 @@ def validate_content( + (" [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: diff --git a/spectacles/validators/content.py b/spectacles/validators/content.py index 390e1f9b..d7d94a14 100644 --- a/spectacles/validators/content.py +++ b/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 diff --git a/tests/test_content_validator.py b/tests/test_content_validator.py index 08ec01bc..904df095 100644 --- a/tests/test_content_validator.py +++ b/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(