Skip to content

Commit

Permalink
remove unnecesary fire_event_if_test (#9522)
Browse files Browse the repository at this point in the history 
* remove unnecesary fire_event_if_test

* modify tests

* remove unused import
  • Loading branch information
@emmyoop
emmyoop authored Feb 7, 2024
1 parent 07726b0 commit 1220fdf
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 270 deletions.
58 changes: 0 additions & 58 deletions core/dbt/parser/models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
from dbt.artifacts.resources import RefArgs
from dbt.context.context_config import ContextConfig
from dbt.contracts.graph.nodes import ModelNode
from dbt_common.events.base_types import EventLevel
from dbt_common.events.types import Note
from dbt_common.events.functions import fire_event_if_test
from dbt.flags import get_flags
from dbt.node_types import NodeType, ModelLanguage
from dbt.parser.base import SimpleSQLParser
Expand Down Expand Up @@ -253,12 +250,6 @@ def render_update(self, node: ModelNode, config: ContextConfig) -> None:
elif not flags.STATIC_PARSER:
# jinja rendering
super().render_update(node, config)
fire_event_if_test(
lambda: Note(
msg=f"1605: jinja rendering because of STATIC_PARSER flag. file: {node.path}"
),
EventLevel.DEBUG,
)
return

# only sample for experimental parser correctness on normal runs,
Expand Down Expand Up @@ -292,10 +283,6 @@ def render_update(self, node: ModelNode, config: ContextConfig) -> None:

# sample the experimental parser only during a normal run
if exp_sample and not flags.USE_EXPERIMENTAL_PARSER:
fire_event_if_test(
lambda: Note(msg=f"1610: conducting experimental parser sample on {node.path}"),
EventLevel.DEBUG,
)
experimental_sample = self.run_experimental_parser(node)
# if the experimental parser succeeded, make a full copy of model parser
# and populate _everything_ into it so it can be compared apples-to-apples
Expand Down Expand Up @@ -325,12 +312,6 @@ def render_update(self, node: ModelNode, config: ContextConfig) -> None:
# sampling rng here, but the effect would be the same since we would only roll
# it 40% of the time. So I've opted to keep all the rng code colocated above.
if stable_sample and not flags.USE_EXPERIMENTAL_PARSER:
fire_event_if_test(
lambda: Note(
msg=f"1611: conducting full jinja rendering sample on {node.path}"
),
EventLevel.DEBUG,
)
# if this will _never_ mutate anything `self` we could avoid these deep copies,
# but we can't really guarantee that going forward.
model_parser_copy = self.partial_deepcopy()
Expand Down Expand Up @@ -365,11 +346,6 @@ def render_update(self, node: ModelNode, config: ContextConfig) -> None:
else:
# jinja rendering
super().render_update(node, config)
# only for test purposes
fire_event_if_test(
lambda: Note(msg=f"1602: parser fallback to jinja rendering on {node.path}"),
EventLevel.DEBUG,
)

# if sampling, add the correct messages for tracking
if exp_sample and isinstance(experimental_sample, str):
Expand Down Expand Up @@ -402,49 +378,23 @@ def render_update(self, node: ModelNode, config: ContextConfig) -> None:
def run_static_parser(self, node: ModelNode) -> Optional[Union[str, Dict[str, List[Any]]]]:
# if any banned macros have been overridden by the user, we cannot use the static parser.
if self._has_banned_macro(node):
# this log line is used for integration testing. If you change
# the code at the beginning of the line change the tests in
# test/integration/072_experimental_parser_tests/test_all_experimental_parser.py
fire_event_if_test(
lambda: Note(
msg=f"1601: detected macro override of ref/source/config in the scope of {node.path}"
),
EventLevel.DEBUG,
)
return "has_banned_macro"

# run the stable static parser and return the results
try:
statically_parsed = py_extract_from_source(node.raw_code)
fire_event_if_test(
lambda: Note(msg=f"1699: static parser successfully parsed {node.path}"),
EventLevel.DEBUG,
)
return _shift_sources(statically_parsed)
# if we want information on what features are barring the static
# parser from reading model files, this is where we would add that
# since that information is stored in the `ExtractionError`.
except ExtractionError:
fire_event_if_test(
lambda: Note(msg=f"1603: static parser failed on {node.path}"),
EventLevel.DEBUG,
)
return "cannot_parse"

def run_experimental_parser(
self, node: ModelNode
) -> Optional[Union[str, Dict[str, List[Any]]]]:
# if any banned macros have been overridden by the user, we cannot use the static parser.
if self._has_banned_macro(node):
# this log line is used for integration testing. If you change
# the code at the beginning of the line change the tests in
# test/integration/072_experimental_parser_tests/test_all_experimental_parser.py
fire_event_if_test(
lambda: Note(
msg=f"1601: detected macro override of ref/source/config in the scope of {node.path}"
),
EventLevel.DEBUG,
)
return "has_banned_macro"

# run the experimental parser and return the results
Expand All @@ -453,19 +403,11 @@ def run_experimental_parser(
# experimental features. Change `py_extract_from_source` to the new
# experimental call when we add additional features.
experimentally_parsed = py_extract_from_source(node.raw_code)
fire_event_if_test(
lambda: Note(msg=f"1698: experimental parser successfully parsed {node.path}"),
EventLevel.DEBUG,
)
return _shift_sources(experimentally_parsed)
# if we want information on what features are barring the experimental
# parser from reading model files, this is where we would add that
# since that information is stored in the `ExtractionError`.
except ExtractionError:
fire_event_if_test(
lambda: Note(msg=f"1604: experimental parser failed on {node.path}"),
EventLevel.DEBUG,
)
return "cannot_parse"

# checks for banned macros
Expand Down
221 changes: 9 additions & 212 deletions tests/functional/experimental_parser/test_all_experimental_parser.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import pytest

from dbt.tests.util import run_dbt, run_dbt_and_capture
from dbt.tests.util import run_dbt

from dbt.artifacts.resources import RefArgs
from dbt.contracts.graph.manifest import Manifest
Expand Down Expand Up @@ -49,49 +49,6 @@ def get_manifest():
"""


ref_macro__schema_yml = """
version: 2
"""

ref_macro__models__model_a_sql = """
select 1 as id
"""

source_macro__macros__source_sql = """
{% macro source(source_name, table_name) %}
{% endmacro %}
"""

source_macro__schema_yml = """
version: 2
"""

source_macro__models__model_a_sql = """
select 1 as id
"""

config_macro__macros__config_sql = """
{% macro config() %}
{% endmacro %}
"""

config_macro__schema_yml = """
version: 2
"""

config_macro__models__model_a_sql = """
select 1 as id
"""


class BasicExperimentalParser:
@pytest.fixture(scope="class")
def models(self):
Expand All @@ -102,54 +59,6 @@ def models(self):
}


class TestBasicExperimentalParserFlag(BasicExperimentalParser):
@pytest.fixture(scope="class", autouse=True)
def setup(self, project):
os.environ["DBT_USE_EXPERIMENTAL_PARSER"] = "true"
yield
del os.environ["DBT_USE_EXPERIMENTAL_PARSER"]

def test_env_use_experimental_parser(self, project):
_, log_output = run_dbt_and_capture(["--debug", "parse"])

# successful stable static parsing
assert not ("1699: " in log_output)
# successful experimental static parsing
assert "1698: " in log_output
# experimental parser failed
assert not ("1604: " in log_output)
# static parser failed
assert not ("1603: " in log_output)
# jinja rendering
assert not ("1602: " in log_output)


class TestBasicStaticParserFlag(BasicExperimentalParser):
@pytest.fixture(scope="class", autouse=True)
def setup(self, project):
os.environ["DBT_STATIC_PARSER"] = "false"
yield
del os.environ["DBT_STATIC_PARSER"]

def test_env_static_parser(self, project):
_, log_output = run_dbt_and_capture(["--debug", "parse"])

print(log_output)

# jinja rendering because of --no-static-parser
assert "1605: " in log_output
# successful stable static parsing
assert not ("1699: " in log_output)
# successful experimental static parsing
assert not ("1698: " in log_output)
# experimental parser failed
assert not ("1604: " in log_output)
# static parser failed
assert not ("1603: " in log_output)
# fallback jinja rendering
assert not ("1602: " in log_output)


class TestBasicExperimentalParser(BasicExperimentalParser):
# test that the experimental parser extracts some basic ref, source, and config calls.
def test_experimental_parser_basic(
Expand All @@ -169,18 +78,7 @@ class TestBasicStaticParser(BasicExperimentalParser):
# test that the static parser extracts some basic ref, source, and config calls by default
# without the experimental flag and without rendering jinja
def test_static_parser_basic(self, project):
_, log_output = run_dbt_and_capture(["--debug", "parse"])

# successful stable static parsing
assert "1699: " in log_output
# successful experimental static parsing
assert not ("1698: " in log_output)
# experimental parser failed
assert not ("1604: " in log_output)
# static parser failed
assert not ("1603: " in log_output)
# jinja rendering
assert not ("1602: " in log_output)
run_dbt(["--debug", "parse"])

manifest = get_manifest()
node = manifest.nodes["model.test.model_a"]
Expand All @@ -193,112 +91,11 @@ def test_static_parser_basic(self, project):
class TestBasicNoStaticParser(BasicExperimentalParser):
# test that the static parser doesn't run when the flag is set
def test_static_parser_is_disabled(self, project):
_, log_output = run_dbt_and_capture(["--debug", "--no-static-parser", "parse"])

# jinja rendering because of --no-static-parser
assert "1605: " in log_output
# successful stable static parsing
assert not ("1699: " in log_output)
# successful experimental static parsing
assert not ("1698: " in log_output)
# experimental parser failed
assert not ("1604: " in log_output)
# static parser failed
assert not ("1603: " in log_output)
# fallback jinja rendering
assert not ("1602: " in log_output)


class TestRefOverrideExperimentalParser:
@pytest.fixture(scope="class")
def models(self):
return {
"model_a.sql": ref_macro__models__model_a_sql,
"schema.yml": ref_macro__schema_yml,
}
run_dbt(["--debug", "--no-static-parser", "parse"])

@pytest.fixture(scope="class")
def macros(self):
return {
"source.sql": source_macro__macros__source_sql,
}

# test that the experimental parser doesn't run if the ref built-in is overriden with a macro
def test_experimental_parser_ref_override(
self,
project,
):
_, log_output = run_dbt_and_capture(["--debug", "--use-experimental-parser", "parse"])

print(log_output)

# successful experimental static parsing
assert not ("1698: " in log_output)
# fallback to jinja rendering
assert "1602: " in log_output
# experimental parser failed
assert not ("1604: " in log_output)
# didn't run static parser because dbt detected a built-in macro override
assert "1601: " in log_output


class TestSourceOverrideExperimentalParser:
@pytest.fixture(scope="class")
def models(self):
return {
"model_a.sql": source_macro__models__model_a_sql,
"schema.yml": source_macro__schema_yml,
}

@pytest.fixture(scope="class")
def macros(self):
return {
"source.sql": source_macro__macros__source_sql,
}

# test that the experimental parser doesn't run if the source built-in is overriden with a macro
def test_experimental_parser_source_override(
self,
project,
):
_, log_output = run_dbt_and_capture(["--debug", "--use-experimental-parser", "parse"])

# successful experimental static parsing
assert not ("1698: " in log_output)
# fallback to jinja rendering
assert "1602: " in log_output
# experimental parser failed
assert not ("1604: " in log_output)
# didn't run static parser because dbt detected a built-in macro override
assert "1601: " in log_output


class TestConfigOverrideExperimentalParser:
@pytest.fixture(scope="class")
def models(self):
return {
"model_a.sql": config_macro__models__model_a_sql,
"schema.yml": config_macro__schema_yml,
}

@pytest.fixture(scope="class")
def macros(self):
return {
"config.sql": config_macro__macros__config_sql,
}

# test that the experimental parser doesn't run if the config built-in is overriden with a macro
def test_experimental_parser_config_override(
self,
project,
):
_, log_output = run_dbt_and_capture(["--debug", "--use-experimental-parser", "parse"])

# successful experimental static parsing
assert not ("1698: " in log_output)
# fallback to jinja rendering
assert "1602: " in log_output
# experimental parser failed
assert not ("1604: " in log_output)
# didn't run static parser because dbt detected a built-in macro override
assert "1601: " in log_output
manifest = get_manifest()
node = manifest.nodes["model.test.model_a"]
assert node.refs == [RefArgs(name="model_b")]
assert node.sources == [["my_src", "my_tbl"]]
assert node.config._extra == {"x": True}
assert node.config.tags == ["hello", "world"]

0 comments on commit 1220fdf

Please sign in to comment.