From de2ec4620c9e3671445f1962ac60ebbf7790dfd4 Mon Sep 17 00:00:00 2001 From: Josh Temple <8672171+joshtemple@users.noreply.github.com> Date: Tue, 11 Feb 2020 23:41:22 -0500 Subject: [PATCH 01/14] Remove async methods from client class --- spectacles/client.py | 1 + 1 file changed, 1 insertion(+) diff --git a/spectacles/client.py b/spectacles/client.py index 7eb4f492..ee130138 100644 --- a/spectacles/client.py +++ b/spectacles/client.py @@ -370,6 +370,7 @@ def create_query_task(self, query_id: int) -> str: logger.debug("Starting query %d", query_id) body = {"query_id": query_id, "result_format": "json_detail"} url = utils.compose_url(self.api_url, path=["query_tasks"]) + response = self.session.post( url=url, json=body, params={"cache": "false"}, timeout=TIMEOUT_SEC ) From bfa87b1f9471102a80a0d600f8594f44551189b2 Mon Sep 17 00:00:00 2001 From: Josh Temple <8672171+joshtemple@users.noreply.github.com> Date: Fri, 14 Feb 2020 16:23:23 -0500 Subject: [PATCH 02/14] Parametrize bad response checks --- tests/test_client.py | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/test_client.py b/tests/test_client.py index cfeed342..5c601d96 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -11,6 +11,43 @@ TEST_CLIENT_SECRET = "test_client_secret" +def get_client_method_names() -> List[str]: + """Extracts method names from LookerClient to test for bad responses""" + client_members: List[Tuple[str, Callable]] = inspect.getmembers( + LookerClient, predicate=inspect.isroutine + ) + client_methods: List[str] = [ + member[0] for member in client_members if not member[0].startswith("__") + ] + for skip_method in ("authenticate", "cancel_query_task"): + client_methods.remove(skip_method) + return client_methods + + +@pytest.fixture +def client_kwargs(): + return dict( + authenticate={ + "client_id": TEST_CLIENT_ID, + "client_secret": TEST_CLIENT_SECRET, + "api_version": 3.1, + }, + get_looker_release_version={}, + update_session={"project": "project_name", "branch": "branch_name"}, + all_lookml_tests={"project": "project_name"}, + run_lookml_test={"project": "project_name"}, + get_lookml_models={}, + get_lookml_dimensions={"model": "model_name", "explore": "explore_name"}, + create_query={ + "model": "model_name", + "explore": "explore_name", + "dimensions": ["dimension_a", "dimension_b"], + }, + create_query_task={"query_id": 13041}, + get_query_task_multi_results={"query_task_ids": ["ajsdkgj", "askkwk"]}, + ) + + def get_client_method_names() -> List[str]: """Extracts method names from LookerClient to test for bad responses""" client_members: List[Tuple[str, Callable]] = inspect.getmembers( From 8991d745e807717d507c3ad08658bb77b25b7569 Mon Sep 17 00:00:00 2001 From: Josh Temple <8672171+joshtemple@users.noreply.github.com> Date: Sun, 16 Feb 2020 22:54:52 -0500 Subject: [PATCH 03/14] Add test to ensure error is set correctly --- tests/test_sql_validator.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/test_sql_validator.py b/tests/test_sql_validator.py index 1cb2aecf..f146699a 100644 --- a/tests/test_sql_validator.py +++ b/tests/test_sql_validator.py @@ -96,6 +96,40 @@ def test_build_project(mock_get_models, mock_get_dimensions, project, validator) assert validator.project == project +# If get_query_results returns an error for a mapped query task ID, +# The corresponding explore should be set to errored and +# The SqlError instance should be present and validated + +# TODO: Refactor error responses into fixtures +# TODO: Should query IDs be ints instead of strings? + + +def test_error_is_set_on_project(project, validator): + query_task_id = "akdk13kkidi2mkv029rld" + message = "An error has occurred" + sql = "SELECT DISTINCT 1 FROM table_name" + error_details = {"message": message, "sql": sql} + validator.project = project + explore = project.models[0].explores[0] + query = Query(query_id="10319", lookml_ref=explore, query_task_id=query_task_id) + validator._running_queries.append(query) + query_result = QueryResult(query_task_id, status="error", error=error_details) + validator._query_by_task_id[query_task_id] = query + returned_sql_error = validator._handle_query_result(query_result) + expected_sql_error = SqlError( + path="test_explore_one", url=None, message=message, sql=sql + ) + assert returned_sql_error == expected_sql_error + assert returned_sql_error == explore.error + assert explore.queried + assert explore.errored + assert validator.project.errored + assert validator.project.models[0].errored + # Batch mode, so none of the dimensions should have errored set + assert not any(dimension.errored for dimension in explore.dimensions) + assert all(dimension.queried for dimension in explore.dimensions) + + def test_get_running_query_tasks(validator): queries = [ Query(query_id="12345", lookml_ref=None, query_task_id="abc"), From 3030609864b7c4d51e8ee9bc557c8215e069cea9 Mon Sep 17 00:00:00 2001 From: Josh Temple <8672171+joshtemple@users.noreply.github.com> Date: Sun, 16 Feb 2020 23:12:01 -0500 Subject: [PATCH 04/14] Test that queries are cancelled --- tests/test_sql_validator.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/test_sql_validator.py b/tests/test_sql_validator.py index f146699a..38d66fa2 100644 --- a/tests/test_sql_validator.py +++ b/tests/test_sql_validator.py @@ -96,15 +96,16 @@ def test_build_project(mock_get_models, mock_get_dimensions, project, validator) assert validator.project == project -# If get_query_results returns an error for a mapped query task ID, -# The corresponding explore should be set to errored and -# The SqlError instance should be present and validated - -# TODO: Refactor error responses into fixtures -# TODO: Should query IDs be ints instead of strings? +def test_error_is_set_on_project(project, validator): + """ + If get_query_results returns an error for a mapped query task ID, + The corresponding explore should be set to errored and + The SqlError instance should be present and validated + TODO: Refactor error responses into fixtures + TODO: Should query IDs be ints instead of strings? -def test_error_is_set_on_project(project, validator): + """ query_task_id = "akdk13kkidi2mkv029rld" message = "An error has occurred" sql = "SELECT DISTINCT 1 FROM table_name" From 638555f540f63226071992dd05f5178a86bb8204 Mon Sep 17 00:00:00 2001 From: Josh Temple <8672171+joshtemple@users.noreply.github.com> Date: Sun, 16 Feb 2020 23:19:39 -0500 Subject: [PATCH 05/14] Test case when queries are still running --- tests/test_sql_validator.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_sql_validator.py b/tests/test_sql_validator.py index 38d66fa2..7d66ac99 100644 --- a/tests/test_sql_validator.py +++ b/tests/test_sql_validator.py @@ -124,6 +124,7 @@ def test_error_is_set_on_project(project, validator): assert returned_sql_error == explore.error assert explore.queried assert explore.errored + assert not validator._running_queries assert validator.project.errored assert validator.project.models[0].errored # Batch mode, so none of the dimensions should have errored set @@ -236,6 +237,22 @@ def test_handle_running_query(validator): assert not returned_sql_error +def test_handle_running_query(validator): + query_task_id = 'sakgwj392jfkajgjcks' + query = Query( + query_id='19428', + lookml_ref=Dimension('dimension_one', 'string', '${TABLE}.dimension_one'), + query_task_id=query_task_id + ) + query_result = QueryResult(query_task_id=query_task_id, status='running') + validator._running_queries = [query] + validator._query_by_task_id[query_task_id] = query + returned_sql_error = validator._handle_query_result(query_result) + + assert validator._running_queries == [query] + assert not returned_sql_error + + def test_count_explores(validator, project): validator.project = project assert validator._count_explores() == 2 From 22a6c28b3d9ddbf9d44601335f65fed8678d59c5 Mon Sep 17 00:00:00 2001 From: Josh Temple <8672171+joshtemple@users.noreply.github.com> Date: Wed, 19 Feb 2020 10:24:19 +0530 Subject: [PATCH 06/14] Fix assertion for cancellation --- tests/test_sql_validator.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_sql_validator.py b/tests/test_sql_validator.py index 7d66ac99..9bded020 100644 --- a/tests/test_sql_validator.py +++ b/tests/test_sql_validator.py @@ -238,13 +238,13 @@ def test_handle_running_query(validator): def test_handle_running_query(validator): - query_task_id = 'sakgwj392jfkajgjcks' + query_task_id = "sakgwj392jfkajgjcks" query = Query( - query_id='19428', - lookml_ref=Dimension('dimension_one', 'string', '${TABLE}.dimension_one'), - query_task_id=query_task_id + query_id="19428", + lookml_ref=Dimension("dimension_one", "string", "${TABLE}.dimension_one"), + query_task_id=query_task_id, ) - query_result = QueryResult(query_task_id=query_task_id, status='running') + query_result = QueryResult(query_task_id=query_task_id, status="running") validator._running_queries = [query] validator._query_by_task_id[query_task_id] = query returned_sql_error = validator._handle_query_result(query_result) From cbcd8053f40cc13f5ea17462a5b77057bd5d4175 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Sat, 21 Mar 2020 17:01:29 -0400 Subject: [PATCH 07/14] add exclude CLI argument to parser --- spectacles/cli.py | 13 ++++++++++++- spectacles/runner.py | 6 +++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/spectacles/cli.py b/spectacles/cli.py index 6f7fb37b..61f15727 100644 --- a/spectacles/cli.py +++ b/spectacles/cli.py @@ -194,6 +194,7 @@ def main(): args.project, args.branch, args.explores, + args.exclude, args.base_url, args.client_id, args.client_secret, @@ -371,6 +372,15 @@ def _build_sql_subparser( The '*' wildcard selects all models or explores. For instance,\ 'model_name/*' would select all explores in the 'model_name' model.", ) + subparser.add_argument( + "--exclude", + nargs="+", + default=[], + help="Specify the explores spectacles should exclude when testing. \ + List of selector strings in 'model_name/explore_name' format. \ + The '*' wildcard selects all models or explores. For instance,\ + 'model_name/*' would select all explores in the 'model_name' model.", + ) subparser.add_argument( "--mode", choices=["batch", "single", "hybrid"], @@ -464,6 +474,7 @@ def run_sql( project, branch, explores, + exclude, base_url, client_id, client_secret, @@ -484,7 +495,7 @@ def run_sql( api_version, remote_reset, ) - errors = runner.validate_sql(explores, mode, concurrency) + errors = runner.validate_sql(explores, exclude, mode, concurrency) if errors: for error in sorted(errors, key=lambda x: x["path"]): printer.print_sql_error(error) diff --git a/spectacles/runner.py b/spectacles/runner.py index 915c3977..ee3357e6 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -40,7 +40,11 @@ def __init__( @log_duration def validate_sql( - self, selectors: List[str], mode: str = "batch", concurrency: int = 10 + self, + selectors: List[str], + exclusions: List[str], + mode: str = "batch", + concurrency: int = 10, ) -> List[dict]: sql_validator = SqlValidator(self.client, self.project, concurrency) sql_validator.build_project(selectors) From fba52f7206ada9518304c992a59b91af006abdb6 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Sat, 21 Mar 2020 17:56:53 -0400 Subject: [PATCH 08/14] exclusion in validator functioning --- spectacles/runner.py | 2 +- spectacles/validators.py | 27 +++++++++++++++++++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/spectacles/runner.py b/spectacles/runner.py index ee3357e6..89d63789 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -47,7 +47,7 @@ def validate_sql( concurrency: int = 10, ) -> List[dict]: sql_validator = SqlValidator(self.client, self.project, concurrency) - sql_validator.build_project(selectors) + sql_validator.build_project(selectors, exclusions) errors = sql_validator.validate(mode) return [vars(error) for error in errors] diff --git a/spectacles/validators.py b/spectacles/validators.py index b3ecd5bd..307827b9 100644 --- a/spectacles/validators.py +++ b/spectacles/validators.py @@ -163,7 +163,7 @@ def _select(self, choices: Sequence[str], select_from: Sequence) -> Sequence: ) return [each for each in select_from if each.name in unique_choices] - def build_project(self, selectors: List[str]) -> None: + def build_project(self, selectors: List[str], exclusions: List[str]) -> None: """Creates an object representation of the project's LookML. Args: @@ -173,6 +173,7 @@ def build_project(self, selectors: List[str]) -> None: """ selection = self.parse_selectors(selectors) + exclusion = self.parse_selectors(exclusions) logger.info( f"Building LookML project hierarchy for project {self.project.name}" ) @@ -194,9 +195,25 @@ def build_project(self, selectors: List[str]) -> None: selected_models = self._select( choices=tuple(selection.keys()), select_from=project_models ) + excluded_models = self._select( + choices=tuple(exclusion.keys()), select_from=project_models + ) - for model in selected_models: + excluded_explores = {} + for model in excluded_models: # Expand wildcard operator to include all specified or discovered explores + excluded_explore_names = exclusion[model.name] + if "*" in excluded_explore_names: + excluded_explore_names.remove("*") + excluded_explore_names.update( + set(explore.name for explore in model.explores) + ) + + excluded_explores[model.name] = self._select( + choices=tuple(excluded_explore_names), select_from=model.explores + ) + + for model in selected_models: selected_explore_names = selection[model.name] if "*" in selected_explore_names: selected_explore_names.remove("*") @@ -207,6 +224,12 @@ def build_project(self, selectors: List[str]) -> None: selected_explores = self._select( choices=tuple(selected_explore_names), select_from=model.explores ) + if model.name in excluded_explores: + selected_explores = [ + explore + for explore in selected_explores + if explore not in excluded_explores[model.name] + ] for explore in selected_explores: dimensions_json = self.client.get_lookml_dimensions( From 2c74fc841c11ccfca9e8afe4b9e16ad4ac5551a4 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Sat, 21 Mar 2020 18:00:13 -0400 Subject: [PATCH 09/14] fix existing test_build_project test --- tests/test_sql_validator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_sql_validator.py b/tests/test_sql_validator.py index 9bded020..f4241f08 100644 --- a/tests/test_sql_validator.py +++ b/tests/test_sql_validator.py @@ -92,7 +92,7 @@ def test_parse_selectors_bad_format_raises_error(): def test_build_project(mock_get_models, mock_get_dimensions, project, validator): mock_get_models.return_value = load("response_models.json") mock_get_dimensions.return_value = load("response_dimensions.json") - validator.build_project(selectors=["*/*"]) + validator.build_project(selectors=["*/*"], exclusions=[]) assert validator.project == project From 52e904519e3e7d58bb0e17f27b2322f1e49fe548 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Sat, 21 Mar 2020 19:25:10 -0400 Subject: [PATCH 10/14] adding tests and removing empty models from project --- spectacles/validators.py | 4 +++- tests/test_sql_validator.py | 42 +++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/spectacles/validators.py b/spectacles/validators.py index 307827b9..015ff708 100644 --- a/spectacles/validators.py +++ b/spectacles/validators.py @@ -243,7 +243,9 @@ def build_project(self, selectors: List[str], exclusions: List[str]) -> None: model.explores = selected_explores - self.project.models = selected_models + self.project.models = [ + model for model in selected_models if len(model.explores) > 0 + ] def validate(self, mode: str = "batch") -> List[SqlError]: """Queries selected explores and returns the project tree with errors.""" diff --git a/tests/test_sql_validator.py b/tests/test_sql_validator.py index f4241f08..168adbfd 100644 --- a/tests/test_sql_validator.py +++ b/tests/test_sql_validator.py @@ -96,6 +96,48 @@ def test_build_project(mock_get_models, mock_get_dimensions, project, validator) assert validator.project == project +@patch("spectacles.client.LookerClient.get_lookml_dimensions") +@patch("spectacles.client.LookerClient.get_lookml_models") +def test_build_project_all_models_excluded( + mock_get_models, mock_get_dimensions, project, validator +): + mock_get_models.return_value = load("response_models.json") + mock_get_dimensions.return_value = load("response_dimensions.json") + validator.build_project( + selectors=["*/*"], exclusions=["test_model_one/*", "test_model.two/*"] + ) + project.models = [] + assert validator.project == project + + +@patch("spectacles.client.LookerClient.get_lookml_dimensions") +@patch("spectacles.client.LookerClient.get_lookml_models") +def test_build_project_one_model_excluded( + mock_get_models, mock_get_dimensions, project, validator +): + mock_get_models.return_value = load("response_models.json") + mock_get_dimensions.return_value = load("response_dimensions.json") + validator.build_project(selectors=["*/*"], exclusions=["test_model_one/*"]) + for model in project.models: + if model.name != "test_model.two": + project.models.remove(model) + assert validator.project == project + + +@patch("spectacles.client.LookerClient.get_lookml_dimensions") +@patch("spectacles.client.LookerClient.get_lookml_models") +def test_build_project_one_model_selected( + mock_get_models, mock_get_dimensions, project, validator +): + mock_get_models.return_value = load("response_models.json") + mock_get_dimensions.return_value = load("response_dimensions.json") + validator.build_project(selectors=["test_model.two/*"], exclusions=[]) + for model in project.models: + if model.name != "test_model.two": + project.models.remove(model) + assert validator.project == project + + def test_error_is_set_on_project(project, validator): """ If get_query_results returns an error for a mapped query task ID, From 231af457581102ed7e2d788ce364728f54983ef1 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Sat, 21 Mar 2020 19:30:20 -0400 Subject: [PATCH 11/14] more tests --- tests/test_sql_validator.py | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/tests/test_sql_validator.py b/tests/test_sql_validator.py index 168adbfd..e74821ad 100644 --- a/tests/test_sql_validator.py +++ b/tests/test_sql_validator.py @@ -119,7 +119,7 @@ def test_build_project_one_model_excluded( mock_get_dimensions.return_value = load("response_dimensions.json") validator.build_project(selectors=["*/*"], exclusions=["test_model_one/*"]) for model in project.models: - if model.name != "test_model.two": + if model.name == "test_model_one": project.models.remove(model) assert validator.project == project @@ -138,6 +138,38 @@ def test_build_project_one_model_selected( assert validator.project == project +@patch("spectacles.client.LookerClient.get_lookml_dimensions") +@patch("spectacles.client.LookerClient.get_lookml_models") +def test_build_project_one_explore_excluded( + mock_get_models, mock_get_dimensions, project, validator +): + mock_get_models.return_value = load("response_models.json") + mock_get_dimensions.return_value = load("response_dimensions.json") + validator.build_project( + selectors=["*/*"], exclusions=["test_model_one/test_explore_one"] + ) + for model in project.models: + if model.name == "test_model_one": + project.models.remove(model) + assert validator.project == project + + +@patch("spectacles.client.LookerClient.get_lookml_dimensions") +@patch("spectacles.client.LookerClient.get_lookml_models") +def test_build_project_one_explore_selected( + mock_get_models, mock_get_dimensions, project, validator +): + mock_get_models.return_value = load("response_models.json") + mock_get_dimensions.return_value = load("response_dimensions.json") + validator.build_project( + selectors=["test_model.two/test_explore_two"], exclusions=[] + ) + for model in project.models: + if model.name != "test_model.two": + project.models.remove(model) + assert validator.project == project + + def test_error_is_set_on_project(project, validator): """ If get_query_results returns an error for a mapped query task ID, From 711062a49104b0362cac120dad3a52ea56b44bc2 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Sun, 29 Mar 2020 11:09:49 -0400 Subject: [PATCH 12/14] fix duped tests: --- tests/test_client.py | 37 -------------------------- tests/test_sql_validator.py | 52 ------------------------------------- 2 files changed, 89 deletions(-) diff --git a/tests/test_client.py b/tests/test_client.py index 5c601d96..cfeed342 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -11,43 +11,6 @@ TEST_CLIENT_SECRET = "test_client_secret" -def get_client_method_names() -> List[str]: - """Extracts method names from LookerClient to test for bad responses""" - client_members: List[Tuple[str, Callable]] = inspect.getmembers( - LookerClient, predicate=inspect.isroutine - ) - client_methods: List[str] = [ - member[0] for member in client_members if not member[0].startswith("__") - ] - for skip_method in ("authenticate", "cancel_query_task"): - client_methods.remove(skip_method) - return client_methods - - -@pytest.fixture -def client_kwargs(): - return dict( - authenticate={ - "client_id": TEST_CLIENT_ID, - "client_secret": TEST_CLIENT_SECRET, - "api_version": 3.1, - }, - get_looker_release_version={}, - update_session={"project": "project_name", "branch": "branch_name"}, - all_lookml_tests={"project": "project_name"}, - run_lookml_test={"project": "project_name"}, - get_lookml_models={}, - get_lookml_dimensions={"model": "model_name", "explore": "explore_name"}, - create_query={ - "model": "model_name", - "explore": "explore_name", - "dimensions": ["dimension_a", "dimension_b"], - }, - create_query_task={"query_id": 13041}, - get_query_task_multi_results={"query_task_ids": ["ajsdkgj", "askkwk"]}, - ) - - def get_client_method_names() -> List[str]: """Extracts method names from LookerClient to test for bad responses""" client_members: List[Tuple[str, Callable]] = inspect.getmembers( diff --git a/tests/test_sql_validator.py b/tests/test_sql_validator.py index e74821ad..cc69dc88 100644 --- a/tests/test_sql_validator.py +++ b/tests/test_sql_validator.py @@ -170,42 +170,6 @@ def test_build_project_one_explore_selected( assert validator.project == project -def test_error_is_set_on_project(project, validator): - """ - If get_query_results returns an error for a mapped query task ID, - The corresponding explore should be set to errored and - The SqlError instance should be present and validated - - TODO: Refactor error responses into fixtures - TODO: Should query IDs be ints instead of strings? - - """ - query_task_id = "akdk13kkidi2mkv029rld" - message = "An error has occurred" - sql = "SELECT DISTINCT 1 FROM table_name" - error_details = {"message": message, "sql": sql} - validator.project = project - explore = project.models[0].explores[0] - query = Query(query_id="10319", lookml_ref=explore, query_task_id=query_task_id) - validator._running_queries.append(query) - query_result = QueryResult(query_task_id, status="error", error=error_details) - validator._query_by_task_id[query_task_id] = query - returned_sql_error = validator._handle_query_result(query_result) - expected_sql_error = SqlError( - path="test_explore_one", url=None, message=message, sql=sql - ) - assert returned_sql_error == expected_sql_error - assert returned_sql_error == explore.error - assert explore.queried - assert explore.errored - assert not validator._running_queries - assert validator.project.errored - assert validator.project.models[0].errored - # Batch mode, so none of the dimensions should have errored set - assert not any(dimension.errored for dimension in explore.dimensions) - assert all(dimension.queried for dimension in explore.dimensions) - - def test_get_running_query_tasks(validator): queries = [ Query(query_id="12345", lookml_ref=None, query_task_id="abc"), @@ -311,22 +275,6 @@ def test_handle_running_query(validator): assert not returned_sql_error -def test_handle_running_query(validator): - query_task_id = "sakgwj392jfkajgjcks" - query = Query( - query_id="19428", - lookml_ref=Dimension("dimension_one", "string", "${TABLE}.dimension_one"), - query_task_id=query_task_id, - ) - query_result = QueryResult(query_task_id=query_task_id, status="running") - validator._running_queries = [query] - validator._query_by_task_id[query_task_id] = query - returned_sql_error = validator._handle_query_result(query_result) - - assert validator._running_queries == [query] - assert not returned_sql_error - - def test_count_explores(validator, project): validator.project = project assert validator._count_explores() == 2 From 7232598270c0c0cfc6142c50f6c50c708edb9c72 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Tue, 31 Mar 2020 08:50:53 -0400 Subject: [PATCH 13/14] Update spectacles/cli.py Co-Authored-By: Josh Temple <8672171+joshtemple@users.noreply.github.com> --- spectacles/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spectacles/cli.py b/spectacles/cli.py index 61f15727..e6ee5ec7 100644 --- a/spectacles/cli.py +++ b/spectacles/cli.py @@ -378,7 +378,7 @@ def _build_sql_subparser( default=[], help="Specify the explores spectacles should exclude when testing. \ List of selector strings in 'model_name/explore_name' format. \ - The '*' wildcard selects all models or explores. For instance,\ + The '*' wildcard excludes all models or explores. For instance,\ 'model_name/*' would select all explores in the 'model_name' model.", ) subparser.add_argument( From 46f8fb9aa56a80e439a3f62ff146bb8fbd4d0053 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Tue, 31 Mar 2020 14:24:26 -0400 Subject: [PATCH 14/14] adding second explore to test models and adding ambiguous explore exclusion test --- tests/resources/response_models.json | 8 ++++ tests/test_sql_validator.py | 56 +++++++++++++++++++++------- 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/tests/resources/response_models.json b/tests/resources/response_models.json index d8cda894..a81df5e2 100644 --- a/tests/resources/response_models.json +++ b/tests/resources/response_models.json @@ -28,6 +28,14 @@ }, { "explores": [ + { + "description": null, + "label": "Test Explore One", + "hidden": false, + "group_label": "Test Explore One", + "name": "test_explore_one", + "can": {} + }, { "description": null, "label": "Test Explore Two", diff --git a/tests/test_sql_validator.py b/tests/test_sql_validator.py index cc69dc88..0e248165 100644 --- a/tests/test_sql_validator.py +++ b/tests/test_sql_validator.py @@ -55,7 +55,10 @@ def project(): ), ] explores_model_one = [Explore("test_explore_one", dimensions)] - explores_model_two = [Explore("test_explore_two", dimensions)] + explores_model_two = [ + Explore("test_explore_one", dimensions), + Explore("test_explore_two", dimensions), + ] models = [ Model("test_model_one", "test_project", explores_model_one), Model("test_model.two", "test_project", explores_model_two), @@ -118,9 +121,9 @@ def test_build_project_one_model_excluded( mock_get_models.return_value = load("response_models.json") mock_get_dimensions.return_value = load("response_dimensions.json") validator.build_project(selectors=["*/*"], exclusions=["test_model_one/*"]) - for model in project.models: - if model.name == "test_model_one": - project.models.remove(model) + project.models = [ + model for model in project.models if model.name != "test_model_one" + ] assert validator.project == project @@ -132,9 +135,9 @@ def test_build_project_one_model_selected( mock_get_models.return_value = load("response_models.json") mock_get_dimensions.return_value = load("response_dimensions.json") validator.build_project(selectors=["test_model.two/*"], exclusions=[]) - for model in project.models: - if model.name != "test_model.two": - project.models.remove(model) + project.models = [ + model for model in project.models if model.name == "test_model.two" + ] assert validator.project == project @@ -148,9 +151,9 @@ def test_build_project_one_explore_excluded( validator.build_project( selectors=["*/*"], exclusions=["test_model_one/test_explore_one"] ) - for model in project.models: - if model.name == "test_model_one": - project.models.remove(model) + project.models = [ + model for model in project.models if model.name != "test_model_one" + ] assert validator.project == project @@ -164,9 +167,34 @@ def test_build_project_one_explore_selected( validator.build_project( selectors=["test_model.two/test_explore_two"], exclusions=[] ) + project.models = [ + model for model in project.models if model.name == "test_model.two" + ] + project.models[0].explores = [ + explore + for explore in project.models[0].explores + if explore.name == "test_explore_two" + ] + assert validator.project == project + + +@patch("spectacles.client.LookerClient.get_lookml_dimensions") +@patch("spectacles.client.LookerClient.get_lookml_models") +def test_build_project_one_ambiguous_explore_excluded( + mock_get_models, mock_get_dimensions, project, validator +): + mock_get_models.return_value = load("response_models.json") + mock_get_dimensions.return_value = load("response_dimensions.json") + validator.build_project( + selectors=["*/*"], exclusions=["test_model.two/test_explore_one"] + ) for model in project.models: - if model.name != "test_model.two": - project.models.remove(model) + if model.name == "test_model.two": + model.explores = [ + explore + for explore in model.explores + if explore.name != "test_explore_one" + ] assert validator.project == project @@ -277,11 +305,11 @@ def test_handle_running_query(validator): def test_count_explores(validator, project): validator.project = project - assert validator._count_explores() == 2 + assert validator._count_explores() == 3 explore = validator.project.models[0].explores[0] validator.project.models[0].explores.extend([explore, explore]) - assert validator._count_explores() == 4 + assert validator._count_explores() == 5 def test_extract_error_details_error_dict(validator):