Fix issue with exclusions in config files and env vars

spectacles/cli.py

@@ -8,7 +8,7 @@
 import argparse
 import logging
 import os
-from typing import Callable, List
+from typing import Callable
 from spectacles import __version__
 from spectacles.runner import Runner
 from spectacles.client import LookerClient
@@ -200,13 +200,14 @@ def wrapper(*args, **kwargs):
     return wrapper
 
 
-def preprocess_dashes(args: List[str]) -> List[str]:
+def preprocess_dash(arg: str) -> str:
     """Replace any dashes with tildes, otherwise argparse will assume they're options"""
-    pattern = re.compile(r"^-(?=([\w_\*]+/[\w_\*]+)|(\d+)$)")
-    processed = []
-    for arg in args:
-        processed.append(pattern.sub("~", arg))
-    return processed
+    return re.sub(r"^-(?=([\w_\*]+/[\w_\*]+)|(\d+)$)", "~", arg)
+
+
+def restore_dash(arg: str) -> str:
+    """Convert leading tildes back to dashes."""
+    return re.sub(r"^~", "-", arg)
 
 
 @handle_exceptions
@@ -219,7 +220,8 @@ def main():
             detail="The current Python version is %s." % platform.python_version(),
         )
 
-    args = preprocess_dashes(sys.argv[1:])
+    # Convert leading `-` to `~` so they don't break `parse_args`
+    args = [preprocess_dash(arg) for arg in sys.argv[1:]]
     parser = create_parser()
     args = parser.parse_args(args)
 
@@ -254,71 +256,71 @@ def main():
 
     if args.command == "connect":
         run_connect(
-            args.base_url,
-            args.client_id,
-            args.client_secret,
-            args.port,
-            args.api_version,
+            base_url=args.base_url,
+            client_id=args.client_id,
+            client_secret=args.client_secret,
+            port=args.port,
+            api_version=args.api_version,
         )
     elif args.command == "sql":
         run_sql(
-            args.log_dir,
-            args.project,
-            ref,
-            args.explores,
-            args.base_url,
-            args.client_id,
-            args.client_secret,
-            args.port,
-            args.api_version,
-            args.fail_fast,
-            args.incremental,
-            args.target,
-            args.remote_reset,
-            args.concurrency,
-            args.profile,
-            args.runtime_threshold,
-            args.chunk_size,
+            log_dir=args.log_dir,
+            project=args.project,
+            ref=ref,
+            filters=[restore_dash(arg) for arg in args.explores],
+            base_url=args.base_url,
+            client_id=args.client_id,
+            client_secret=args.client_secret,
+            port=args.port,
+            api_version=args.api_version,
+            fail_fast=args.fail_fast,
+            incremental=args.incremental,
+            target=args.target,
+            remote_reset=args.remote_reset,
+            concurrency=args.concurrency,
+            profile=args.profile,
+            runtime_threshold=args.runtime_threshold,
+            chunk_size=args.chunk_size,
         )
     elif args.command == "assert":
         run_assert(
-            args.project,
-            ref,
-            args.explores,
-            args.base_url,
-            args.client_id,
-            args.client_secret,
-            args.port,
-            args.api_version,
-            args.remote_reset,
+            project=args.project,
+            ref=ref,
+            filters=[restore_dash(arg) for arg in args.explores],
+            base_url=args.base_url,
+            client_id=args.client_id,
+            client_secret=args.client_secret,
+            port=args.port,
+            api_version=args.api_version,
+            remote_reset=args.remote_reset,
         )
     elif args.command == "content":
         run_content(
-            args.project,
-            ref,
-            args.explores,
-            args.base_url,
-            args.client_id,
-            args.client_secret,
-            args.port,
-            args.api_version,
-            args.remote_reset,
-            args.incremental,
-            args.target,
-            args.exclude_personal,
-            args.folders,
+            project=args.project,
+            ref=ref,
+            filters=[restore_dash(arg) for arg in args.explores],
+            base_url=args.base_url,
+            client_id=args.client_id,
+            client_secret=args.client_secret,
+            port=args.port,
+            api_version=args.api_version,
+            remote_reset=args.remote_reset,
+            incremental=args.incremental,
+            target=args.target,
+            exclude_personal=args.exclude_personal,
+            folders=[restore_dash(arg) for arg in args.folders],
         )
     elif args.command == "lookml":
         run_lookml(
-            args.project,
-            ref,
-            args.base_url,
-            args.client_id,
-            args.client_secret,
-            args.port,
-            args.api_version,
-            args.remote_reset,
-            args.severity,
+            project=args.project,
+            ref=ref,
+            base_url=args.base_url,
+            client_id=args.client_id,
+            client_secret=args.client_secret,
+            port=args.port,
+            api_version=args.api_version,
+            remote_reset=args.remote_reset,
+            severity=args.severity,
         )
 
     if not args.do_not_track:

spectacles/select.py

@@ -31,7 +31,7 @@ def is_selected(model: str, explore: str, filters: List[str]) -> bool:
     included = None
     for f in filters:
         # If it matches an exclude, stop immediately
-        if f[0] == "~":
+        if f[0] == "-":
             if re.match(selector_to_pattern(f[1:]), test_string):
                 return False
         elif included:

spectacles/validators/content.py

@@ -17,7 +17,7 @@ def __init__(
         exclude_folders = []
         if folders:
             for folder_id in folders:
-                if folder_id.startswith("~"):
+                if folder_id.startswith("-"):
                     exclude_folders.append(int(folder_id[1:]))
                 else:
                     include_folders.append(int(folder_id))

tests/test_cli.py

@@ -4,7 +4,7 @@
 import requests
 from constants import ENV_VARS
 from utils import build_validation
-from spectacles.cli import main, create_parser, handle_exceptions, preprocess_dashes
+from spectacles.cli import main, create_parser, handle_exceptions, preprocess_dash
 from spectacles.exceptions import (
     LookerApiError,
     SpectaclesException,
@@ -142,11 +142,14 @@ def test_parse_args_with_only_config_file(mock_parse_config, clean_env):
         "base_url": "BASE_URL_CONFIG",
         "client_id": "CLIENT_ID_CONFIG",
         "client_secret": "CLIENT_SECRET_CONFIG",
+        "project": "spectacles",
+        "explores": ["model_a/*", "-model_a/explore_b"],
     }
-    args = parser.parse_args(["connect", "--config-file", "config.yml"])
+    args = parser.parse_args(["sql", "--config-file", "config.yml"])
     assert args.base_url == "BASE_URL_CONFIG"
     assert args.client_id == "CLIENT_ID_CONFIG"
     assert args.client_secret == "CLIENT_SECRET_CONFIG"
+    assert args.explores == ["model_a/*", "-model_a/explore_b"]
 
 
 @patch("spectacles.cli.YamlConfigAction.parse_config")
@@ -162,6 +165,54 @@ def test_parse_args_with_incomplete_config_file(mock_parse_config, clean_env, ca
     assert "the following arguments are required: --client-secret" in captured.err
 
 
+@patch("spectacles.cli.run_sql")
+@patch("spectacles.cli.YamlConfigAction.parse_config")
+def test_config_file_explores_folders_processed_correctly(
+    mock_parse_config, mock_run_sql, clean_env
+):
+    mock_parse_config.return_value = {
+        "base_url": "BASE_URL_CONFIG",
+        "client_id": "CLIENT_ID_CONFIG",
+        "client_secret": "CLIENT_SECRET_CONFIG",
+        "project": "spectacles",
+        "explores": ["model_a/*", "-model_a/explore_b"],
+    }
+    with patch("sys.argv", ["spectacles", "sql", "--config-file", "config.yml"]):
+        main()
+
+    assert mock_run_sql.call_args[1]["filters"] == [
+        "model_a/*",
+        "-model_a/explore_b",
+    ]
+
+
+@patch("spectacles.cli.run_sql")
+def test_cli_explores_folders_processed_correctly(mock_run_sql, clean_env):
+    with patch(
+        "sys.argv",
+        [
+            "spectacles",
+            "sql",
+            "--base-url",
+            "BASE_URL",
+            "--client-id",
+            "CLIENT_ID",
+            "--client-secret",
+            "CLIENT_SECRET",
+            "--project",
+            "spectacles",
+            "--explores",
+            "model_a/*",
+            "-model_a/explore_b",
+        ],
+    ):
+        main()
+    assert mock_run_sql.call_args[1]["filters"] == [
+        "model_a/*",
+        "-model_a/explore_b",
+    ]
+
+
 def test_parse_args_with_only_env_vars(env):
     parser = create_parser()
     args = parser.parse_args(["connect"])
@@ -351,11 +402,11 @@ def test_main_with_connect(mock_tracking, mock_run_connect, env):
     )
     mock_tracking.track_invocation_end.assert_called_once()
     mock_run_connect.assert_called_once_with(
-        "BASE_URL_ENV_VAR",  # base_url
-        "CLIENT_ID_ENV_VAR",  # client_id
-        "CLIENT_SECRET_ENV_VAR",  # client_secret
-        8080,  # port
-        3.1,  # api_version
+        base_url="BASE_URL_ENV_VAR",
+        client_id="CLIENT_ID_ENV_VAR",
+        client_secret="CLIENT_SECRET_ENV_VAR",
+        port=8080,
+        api_version=3.1,
     )
 
 
@@ -367,29 +418,33 @@ def test_main_with_do_not_track(mock_tracking, mock_run_connect, env):
     mock_tracking.track_invocation_start.assert_not_called()
     mock_tracking.track_invocation_end.assert_not_called()
     mock_run_connect.assert_called_once_with(
-        "BASE_URL_ENV_VAR",  # base_url
-        "CLIENT_ID_ENV_VAR",  # client_id
-        "CLIENT_SECRET_ENV_VAR",  # client_secret
-        8080,  # port
-        3.1,  # api_version
+        base_url="BASE_URL_ENV_VAR",
+        client_id="CLIENT_ID_ENV_VAR",
+        client_secret="CLIENT_SECRET_ENV_VAR",
+        port=8080,
+        api_version=3.1,
     )
 
 
 def test_preprocess_dashes_with_folder_ids_should_work():
-    args = preprocess_dashes(["--folders", "40", "25", "-41", "-1", "-344828", "3929"])
+    args = [
+        preprocess_dash(arg)
+        for arg in ["--folders", "40", "25", "-41", "-1", "-344828", "3929"]
+    ]
     assert args == ["--folders", "40", "25", "~41", "~1", "~344828", "3929"]
 
 
 def test_preprocess_dashes_with_model_explores_should_work():
-    args = preprocess_dashes(
-        [
+    args = [
+        preprocess_dash(arg)
+        for arg in [
             "--explores",
             "model_a/explore_a",
             "-model_b/explore_b",
             "model_c/explore_c",
             "-model_d/explore_d",
         ]
-    )
+    ]
     assert args == [
         "--explores",
         "model_a/explore_a",
@@ -400,19 +455,22 @@ def test_preprocess_dashes_with_model_explores_should_work():
 
 
 def test_preprocess_dashes_with_wildcards_should_work():
-    args = preprocess_dashes(
-        [
+    args = [
+        preprocess_dash(arg)
+        for arg in (
             "--explores",
             "*/explore_a",
             "-model_b/*",
+            "model-a/explore-a",
             "*/*",
             "-*/*",
-        ]
-    )
+        )
+    ]
     assert args == [
         "--explores",
         "*/explore_a",
         "~model_b/*",
+        "model-a/explore-a",
         "*/*",
         "~*/*",
     ]

tests/test_select.py

@@ -48,21 +48,21 @@ def test_select_wrong_explore_should_not_match():
     assert not is_selected("model_a", "explore_a", ["model_a/explore_b"])
 
 
-@pytest.mark.parametrize("filters", permutations(["*/*", "~*/*"]))
+@pytest.mark.parametrize("filters", permutations(["*/*", "-*/*"]))
 def test_exclude_wildcard_should_not_match(filters):
     assert not is_selected("model_a", "explore_a", filters)
 
 
-@pytest.mark.parametrize("filters", permutations(["*/*", "~model_a/*"]))
+@pytest.mark.parametrize("filters", permutations(["*/*", "-model_a/*"]))
 def test_exclude_model_wildcard_should_not_match(filters):
     assert not is_selected("model_a", "explore_a", filters)
 
 
-@pytest.mark.parametrize("filters", permutations(["*/*", "~*/explore_a"]))
+@pytest.mark.parametrize("filters", permutations(["*/*", "-*/explore_a"]))
 def test_exclude_explore_wildcard_should_not_match(filters):
     assert not is_selected("model_a", "explore_a", filters)
 
 
-@pytest.mark.parametrize("filters", permutations(["*/*", "~model_a/explore_a"]))
+@pytest.mark.parametrize("filters", permutations(["*/*", "-model_a/explore_a"]))
 def test_exclude_exact_model_and_explore_should_not_match(filters):
     assert not is_selected("model_a", "explore_a", filters)