Skip to content

Commit

Permalink
Rm schema, data flag + tag behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
jtcohen6 committed Sep 14, 2021
1 parent 8f76028 commit 2a9fdfb
Show file tree
Hide file tree
Showing 15 changed files with 21 additions and 112 deletions.
1 change: 0 additions & 1 deletion core/dbt/graph/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
)
from .cli import ( # noqa: F401
parse_difference,
parse_test_selectors,
parse_from_selectors_definition,
)
from .queue import GraphQueue # noqa: F401
Expand Down
37 changes: 0 additions & 37 deletions core/dbt/graph/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@
DEFAULT_INCLUDES: List[str] = ['fqn:*', 'source:*', 'exposure:*']
DEFAULT_EXCLUDES: List[str] = []

# here for backwards compatibility
# we have since renamed data --> singular, schema --> generic
DATA_TEST_SELECTOR: str = 'test_type:data'
SCHEMA_TEST_SELECTOR: str = 'test_type:schema'


def parse_union(
components: List[str], expect_exists: bool, greedy: bool = False
Expand Down Expand Up @@ -73,38 +68,6 @@ def parse_difference(
excluded = parse_union_from_default(exclude, DEFAULT_EXCLUDES, greedy=True)
return SelectionDifference(components=[included, excluded])

# serves to handle --schema and --data flags
# only supported for backwards compatibility
def parse_test_selectors(
data: bool, schema: bool, base: SelectionSpec
) -> SelectionSpec:
union_components = []

if data:
union_components.append(
SelectionCriteria.from_single_spec(DATA_TEST_SELECTOR)
)
if schema:
union_components.append(
SelectionCriteria.from_single_spec(SCHEMA_TEST_SELECTOR)
)

intersect_with: SelectionSpec
if not union_components:
return base
elif len(union_components) == 1:
intersect_with = union_components[0]
else: # data and schema tests
intersect_with = SelectionUnion(
components=union_components,
expect_exists=True,
raw=[DATA_TEST_SELECTOR, SCHEMA_TEST_SELECTOR],
)

return SelectionIntersection(
components=[base, intersect_with], expect_exists=True
)


RawDefinition = Union[str, Dict[str, Any]]

Expand Down
16 changes: 0 additions & 16 deletions core/dbt/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -698,22 +698,6 @@ def _build_test_subparser(subparsers, base_subparser):
Runs tests on data in deployed models. Run this after `dbt run`
'''
)
sub.add_argument(
# here for backwards compatibility
'--data',
action='store_true',
help='''
Run data tests defined in "tests" directory.
'''
)
sub.add_argument(
# here for backwards compatibility
'--schema',
action='store_true',
help='''
Run constraint validations from schema.yml files
'''
)
sub.add_argument(
'-x',
'--fail-fast',
Expand Down
2 changes: 0 additions & 2 deletions core/dbt/parser/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,6 @@ def _parse_generic_test(
'kwargs': builder.args,
}
tags = sorted(set(itertools.chain(tags, builder.tags())))
if 'schema' not in tags:
tags.append('schema')

node = self.create_test_node(
target=target,
Expand Down
6 changes: 0 additions & 6 deletions core/dbt/parser/singular_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,6 @@ def parse_from_dict(self, dct, validate=True) -> ParsedSingularTestNode:
def resource_type(self) -> NodeType:
return NodeType.Test

def transform(self, node):
# here for backwards compatibility only
if 'data' not in node.tags:
node.tags.append('data')
return node

@classmethod
def get_compiled_path(cls, block: FileBlock):
return get_pseudo_test_path(block.name, block.path.relative_path,
Expand Down
12 changes: 0 additions & 12 deletions core/dbt/task/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
)
from dbt.graph import (
ResourceTypeSelector,
SelectionSpec,
parse_test_selectors,
)
from dbt.node_types import NodeType, RunHookType
from dbt import flags
Expand Down Expand Up @@ -173,16 +171,6 @@ def safe_run_hooks(
# Don't execute on-run-* hooks for tests
pass

def get_selection_spec(self) -> SelectionSpec:
base_spec = super().get_selection_spec()
# serves to handle --schema and --data flags
# only supported for backwards compatibility
return parse_test_selectors(
data=self.args.data,
schema=self.args.schema,
base=base_spec
)

def get_node_selector(self) -> TestSelector:
if self.manifest is None or self.graph is None:
raise InternalException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def test_snapshot_check_cols_cycle(self):
self.assertEqual(len(results), 1)

def assert_expected(self):
self.run_dbt(['test', '--data', '--vars', 'version: 3'])
self.run_dbt(['test', '--select', 'test_type:singular', '--vars', 'version: 3'])

@use_profile('snowflake')
def test__snowflake__simple_snapshot(self):
Expand Down
1 change: 0 additions & 1 deletion test/integration/009_data_tests_test/test_data_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ def models(self):

def run_data_validations(self):
args = FakeArgs()
args.data = True

test_task = TestTask(args, self.config)
return test_task.run()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1361,7 +1361,7 @@ def expected_seeded_manifest(self, model_database=None, quote_model=False):
'root_path': self.test_root_realpath,
'schema': test_audit_schema,
'database': self.default_database,
'tags': ['schema'],
'tags': [],
'meta': {},
'unique_id': 'test.test.not_null_model_id.d01cc630e6',
'docs': {'show': True},
Expand Down Expand Up @@ -1452,7 +1452,7 @@ def expected_seeded_manifest(self, model_database=None, quote_model=False):
'root_path': self.test_root_realpath,
'schema': test_audit_schema,
'database': self.default_database,
'tags': ['schema'],
'tags': [],
'meta': {},
'unique_id': 'test.test.test_nothing_model_.5d38568946',
'docs': {'show': True},
Expand Down Expand Up @@ -1498,7 +1498,7 @@ def expected_seeded_manifest(self, model_database=None, quote_model=False):
'root_path': self.test_root_realpath,
'schema': test_audit_schema,
'database': self.default_database,
'tags': ['schema'],
'tags': [],
'meta': {},
'unique_id': 'test.test.unique_model_id.67b76558ff',
'docs': {'show': True},
Expand Down
12 changes: 6 additions & 6 deletions test/integration/045_test_severity_tests/test_severity.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def test_postgres_severity_warnings(self):
self.run_dbt_with_vars(['seed'], 'false', strict=False)
self.run_dbt_with_vars(['run'], 'false', strict=False)
results = self.run_dbt_with_vars(
['test', '--schema'], 'false', strict=False)
['test', '--select', 'test_type:generic'], 'false', strict=False)
self.assertEqual(len(results), 2)
self.assertEqual(results[0].status, 'warn')
self.assertEqual(results[0].failures, 2)
Expand All @@ -43,7 +43,7 @@ def test_postgres_severity_rendered_errors(self):
self.run_dbt_with_vars(['seed'], 'false', strict=False)
self.run_dbt_with_vars(['run'], 'false', strict=False)
results = self.run_dbt_with_vars(
['test', '--schema'], 'true', strict=False, expect_pass=False)
['test', '--select', 'test_type:generic'], 'true', strict=False, expect_pass=False)
self.assertEqual(len(results), 2)
self.assertEqual(results[0].status, 'fail')
self.assertEqual(results[0].failures, 2)
Expand All @@ -55,7 +55,7 @@ def test_postgres_severity_warnings_strict(self):
self.run_dbt_with_vars(['seed'], 'false', strict=False)
self.run_dbt_with_vars(['run'], 'false', strict=False)
results = self.run_dbt_with_vars(
['test', '--schema'], 'false', expect_pass=False)
['test', '--select', 'test_type:generic'], 'false', expect_pass=False)
self.assertEqual(len(results), 2)
self.assertEqual(results[0].status, 'fail')
self.assertEqual(results[0].failures, 2)
Expand All @@ -67,7 +67,7 @@ def test_postgres_data_severity_warnings(self):
self.run_dbt_with_vars(['seed'], 'false', strict=False)
self.run_dbt_with_vars(['run'], 'false', strict=False)
results = self.run_dbt_with_vars(
['test', '--data'], 'false', strict=False)
['test', '--select', 'test_type:singular'], 'false', strict=False)
self.assertEqual(len(results), 1)
self.assertEqual(results[0].status, 'warn')
self.assertEqual(results[0].failures, 2)
Expand All @@ -77,7 +77,7 @@ def test_postgres_data_severity_rendered_errors(self):
self.run_dbt_with_vars(['seed'], 'false', strict=False)
self.run_dbt_with_vars(['run'], 'false', strict=False)
results = self.run_dbt_with_vars(
['test', '--data'], 'true', strict=False, expect_pass=False)
['test', '--select', 'test_type:singular'], 'true', strict=False, expect_pass=False)
self.assertEqual(len(results), 1)
self.assertEqual(results[0].status, 'fail')
self.assertEqual(results[0].failures, 2)
Expand All @@ -87,7 +87,7 @@ def test_postgres_data_severity_warnings_strict(self):
self.run_dbt_with_vars(['seed'], 'false', strict=False)
self.run_dbt_with_vars(['run'], 'false', strict=False)
results = self.run_dbt_with_vars(
['test', '--data'], 'false', expect_pass=False)
['test', '--select', 'test_type:singular'], 'false', expect_pass=False)
self.assertEqual(len(results), 1)
self.assertTrue(results[0].status, 'fail')
self.assertEqual(results[0].failures, 2)
6 changes: 3 additions & 3 deletions test/integration/047_dbt_ls_test/test_ls.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ def expect_test_output(self):
'name': 'not_null_outer_id',
'package_name': 'test',
'depends_on': {'nodes': ['model.test.outer'], 'macros': ['macro.dbt.test_not_null']},
'tags': ['schema'],
'tags': [],
'config': {
'enabled': True,
'materialized': 'test',
Expand All @@ -383,7 +383,7 @@ def expect_test_output(self):
'name': 't',
'package_name': 'test',
'depends_on': {'nodes': [], 'macros': []},
'tags': ['data'],
'tags': [],
'config': {
'enabled': True,
'materialized': 'test',
Expand All @@ -409,7 +409,7 @@ def expect_test_output(self):
'name': 'unique_outer_id',
'package_name': 'test',
'depends_on': {'nodes': ['model.test.outer'], 'macros': ['macro.dbt.test_unique']},
'tags': ['schema'],
'tags': [],
'config': {
'enabled': True,
'materialized': 'test',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ def list_tests_and_assert(self, include, exclude, expected_tests):
test_names = [name.split('.')[2] for name in listed]
assert sorted(test_names) == sorted(expected_tests)

def run_tests_and_assert(
self, include, exclude, expected_tests, schema = False, data = False
):
def run_tests_and_assert(self, include, exclude, expected_tests):
results = self.run_dbt(['run'])
self.assertEqual(len(results), 2)

Expand All @@ -44,10 +42,6 @@ def run_tests_and_assert(
test_args.extend(('--models', include))
if exclude:
test_args.extend(('--exclude', exclude))
if schema:
test_args.append('--schema')
if data:
test_args.append('--data')

results = self.run_dbt(test_args)
tests_run = [r.node.name for r in results]
Expand Down Expand Up @@ -139,7 +133,6 @@ def test__postgres__only_schema(self):

self.list_tests_and_assert(select, exclude, expected)
self.run_tests_and_assert(select, exclude, expected)
self.run_tests_and_assert(None, exclude, expected, schema=True)

@use_profile('postgres')
def test__postgres__model_a_only_data(self):
Expand All @@ -149,7 +142,6 @@ def test__postgres__model_a_only_data(self):

self.list_tests_and_assert(select, exclude, expected)
self.run_tests_and_assert(select, exclude, expected)
self.run_tests_and_assert('model_a', exclude, expected, schema=True)

@use_profile('postgres')
def test__postgres__only_data(self):
Expand All @@ -159,7 +151,6 @@ def test__postgres__only_data(self):

self.list_tests_and_assert(select, exclude, expected)
self.run_tests_and_assert(select, exclude, expected)
self.run_tests_and_assert(None, exclude, expected, data=True)

@use_profile('postgres')
def test__postgres__model_a_only_data(self):
Expand All @@ -169,7 +160,6 @@ def test__postgres__model_a_only_data(self):

self.list_tests_and_assert(select, exclude, expected)
self.run_tests_and_assert(select, exclude, expected)
self.run_tests_and_assert('model_a', exclude, expected, data=True)

@use_profile('postgres')
def test__postgres__test_name_intersection(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def list_tests_and_assert(self, include, exclude, expected_tests):
assert sorted(test_names) == sorted(expected_tests)

def run_tests_and_assert(
self, include, exclude, expected_tests, compare_source, compare_target, schema = False, data = False
self, include, exclude, expected_tests, compare_source, compare_target
):

run_args = ['run']
Expand All @@ -50,10 +50,6 @@ def run_tests_and_assert(
test_args.extend(('--models', include))
if exclude:
test_args.extend(('--exclude', exclude))
if schema:
test_args.append('--schema')
if data:
test_args.append('--data')

results = self.run_dbt(test_args)
tests_run = [r.node.name for r in results]
Expand Down
2 changes: 0 additions & 2 deletions test/integration/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ def __eq__(self, other):
class FakeArgs:
def __init__(self):
self.threads = 1
self.data = False
self.defer = False
self.schema = True
self.full_refresh = False
self.models = None
self.select = None
Expand Down
12 changes: 6 additions & 6 deletions test/unit/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,12 +327,12 @@ def test__parse_basic_source_tests(self):
tests.sort(key=lambda n: n.unique_id)

self.assertEqual(tests[0].config.severity, 'ERROR')
self.assertEqual(tests[0].tags, ['schema'])
self.assertEqual(tests[0].tags, [])
self.assertEqual(tests[0].sources, [['my_source', 'my_table']])
self.assertEqual(tests[0].column_name, 'color')
self.assertEqual(tests[0].fqn, ['snowplow', 'test', tests[0].name])
self.assertEqual(tests[1].config.severity, 'WARN')
self.assertEqual(tests[1].tags, ['schema'])
self.assertEqual(tests[1].tags, [])
self.assertEqual(tests[1].sources, [['my_source', 'my_table']])
self.assertEqual(tests[1].column_name, 'color')
self.assertEqual(tests[1].fqn, ['snowplow', 'test', tests[1].name])
Expand Down Expand Up @@ -409,7 +409,7 @@ def test__parse_basic_model_tests(self):
continue
tests.append(node)
self.assertEqual(tests[0].config.severity, 'ERROR')
self.assertEqual(tests[0].tags, ['schema'])
self.assertEqual(tests[0].tags, [])
self.assertEqual(tests[0].refs, [['my_model']])
self.assertEqual(tests[0].column_name, 'color')
self.assertEqual(tests[0].package_name, 'snowplow')
Expand All @@ -430,7 +430,7 @@ def test__parse_basic_model_tests(self):
# foreign packages are a bit weird, they include the macro package
# name in the test name
self.assertEqual(tests[1].config.severity, 'ERROR')
self.assertEqual(tests[1].tags, ['schema'])
self.assertEqual(tests[1].tags, [])
self.assertEqual(tests[1].refs, [['my_model']])
self.assertEqual(tests[1].column_name, 'color')
self.assertEqual(tests[1].column_name, 'color')
Expand All @@ -450,7 +450,7 @@ def test__parse_basic_model_tests(self):
)

self.assertEqual(tests[2].config.severity, 'WARN')
self.assertEqual(tests[2].tags, ['schema'])
self.assertEqual(tests[2].tags, [])
self.assertEqual(tests[2].refs, [['my_model']])
self.assertEqual(tests[2].column_name, 'color')
self.assertEqual(tests[2].package_name, 'snowplow')
Expand Down Expand Up @@ -786,7 +786,7 @@ def test_basic(self):
root_path=get_abs_os_path('./dbt_modules/snowplow'),
refs=[['blah']],
config=TestConfig(severity='ERROR'),
tags=['data'],
tags=[],
path=normalize('test/test_1.sql'),
raw_sql=raw_sql,
checksum=block.file.checksum,
Expand Down

0 comments on commit 2a9fdfb

Please sign in to comment.