Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Fix #9005: Allow singular tests to be documented in properties.yml" #10790

Merged
merged 1 commit into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions .changes/unreleased/Fixes-20240923-190758.yaml

This file was deleted.

13 changes: 9 additions & 4 deletions core/dbt/config/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,14 @@ def _parse_versions(versions: Union[List[str], str]) -> List[VersionSpecifier]:
return [VersionSpecifier.from_version_string(v) for v in versions]


def _all_source_paths(*args: List[str]) -> List[str]:
paths = chain(*args)
def _all_source_paths(
model_paths: List[str],
seed_paths: List[str],
snapshot_paths: List[str],
analysis_paths: List[str],
macro_paths: List[str],
) -> List[str]:
paths = chain(model_paths, seed_paths, snapshot_paths, analysis_paths, macro_paths)
# Strip trailing slashes since the path is the same even though the name is not
stripped_paths = map(lambda s: s.rstrip("/"), paths)
return list(set(stripped_paths))
Expand Down Expand Up @@ -403,7 +409,7 @@ def create_project(self, rendered: RenderComponents) -> "Project":
snapshot_paths: List[str] = value_or(cfg.snapshot_paths, ["snapshots"])

all_source_paths: List[str] = _all_source_paths(
model_paths, seed_paths, snapshot_paths, analysis_paths, macro_paths, test_paths
model_paths, seed_paths, snapshot_paths, analysis_paths, macro_paths
)

docs_paths: List[str] = value_or(cfg.docs_paths, all_source_paths)
Expand Down Expand Up @@ -646,7 +652,6 @@ def all_source_paths(self) -> List[str]:
self.snapshot_paths,
self.analysis_paths,
self.macro_paths,
self.test_paths,
)

@property
Expand Down
50 changes: 1 addition & 49 deletions core/dbt/contracts/graph/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
SavedQuery,
SeedNode,
SemanticModel,
SingularTestNode,
SourceDefinition,
UnitTestDefinition,
UnitTestFileFixture,
Expand Down Expand Up @@ -90,7 +89,7 @@
RefName = str


def find_unique_id_for_package(storage, key, package: Optional[PackageName]) -> Optional[UniqueID]:
def find_unique_id_for_package(storage, key, package: Optional[PackageName]):
if key not in storage:
return None

Expand Down Expand Up @@ -471,43 +470,6 @@ class AnalysisLookup(RefableLookup):
_versioned_types: ClassVar[set] = set()


class SingularTestLookup(dbtClassMixin):
def __init__(self, manifest: "Manifest") -> None:
self.storage: Dict[str, Dict[PackageName, UniqueID]] = {}
self.populate(manifest)

def get_unique_id(self, search_name, package: Optional[PackageName]) -> Optional[UniqueID]:
return find_unique_id_for_package(self.storage, search_name, package)

def find(
self, search_name, package: Optional[PackageName], manifest: "Manifest"
) -> Optional[SingularTestNode]:
unique_id = self.get_unique_id(search_name, package)
if unique_id is not None:
return self.perform_lookup(unique_id, manifest)
return None

def add_singular_test(self, source: SingularTestNode) -> None:
if source.search_name not in self.storage:
self.storage[source.search_name] = {}

self.storage[source.search_name][source.package_name] = source.unique_id

def populate(self, manifest: "Manifest") -> None:
for node in manifest.nodes.values():
if isinstance(node, SingularTestNode):
self.add_singular_test(node)

def perform_lookup(self, unique_id: UniqueID, manifest: "Manifest") -> SingularTestNode:
if unique_id not in manifest.nodes:
raise dbt_common.exceptions.DbtInternalError(
f"Singular test {unique_id} found in cache but not found in manifest"
)
node = manifest.nodes[unique_id]
assert isinstance(node, SingularTestNode)
return node


def _packages_to_search(
current_project: str,
node_package: str,
Expand Down Expand Up @@ -907,9 +869,6 @@ class Manifest(MacroMethods, dbtClassMixin):
_analysis_lookup: Optional[AnalysisLookup] = field(
default=None, metadata={"serialize": lambda x: None, "deserialize": lambda x: None}
)
_singular_test_lookup: Optional[SingularTestLookup] = field(
default=None, metadata={"serialize": lambda x: None, "deserialize": lambda x: None}
)
_parsing_info: ParsingInfo = field(
default_factory=ParsingInfo,
metadata={"serialize": lambda x: None, "deserialize": lambda x: None},
Expand Down Expand Up @@ -1305,12 +1264,6 @@ def analysis_lookup(self) -> AnalysisLookup:
self._analysis_lookup = AnalysisLookup(self)
return self._analysis_lookup

@property
def singular_test_lookup(self) -> SingularTestLookup:
if self._singular_test_lookup is None:
self._singular_test_lookup = SingularTestLookup(self)
return self._singular_test_lookup

@property
def external_node_unique_ids(self):
return [node.unique_id for node in self.nodes.values() if node.is_external_node]
Expand Down Expand Up @@ -1755,7 +1708,6 @@ def __reduce_ex__(self, protocol):
self._semantic_model_by_measure_lookup,
self._disabled_lookup,
self._analysis_lookup,
self._singular_test_lookup,
)
return self.__class__, args

Expand Down
5 changes: 0 additions & 5 deletions core/dbt/contracts/graph/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1642,11 +1642,6 @@ class ParsedMacroPatch(ParsedPatch):
arguments: List[MacroArgument] = field(default_factory=list)


@dataclass
class ParsedSingularTestPatch(ParsedPatch):
pass


# ====================================
# Node unions/categories
# ====================================
Expand Down
5 changes: 0 additions & 5 deletions core/dbt/contracts/graph/unparsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,6 @@ class UnparsedAnalysisUpdate(HasConfig, HasColumnDocs, HasColumnProps, HasYamlMe
access: Optional[str] = None


@dataclass
class UnparsedSingularTestUpdate(HasConfig, HasColumnProps, HasYamlMetadata):
pass


@dataclass
class UnparsedNodeUpdate(HasConfig, HasColumnTests, HasColumnAndTestProps, HasYamlMetadata):
quote_columns: Optional[bool] = None
Expand Down
2 changes: 0 additions & 2 deletions core/dbt/parser/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
UnparsedMacroUpdate,
UnparsedModelUpdate,
UnparsedNodeUpdate,
UnparsedSingularTestUpdate,
)
from dbt.exceptions import ParsingError
from dbt.node_types import NodeType
Expand Down Expand Up @@ -59,7 +58,6 @@ def trimmed(inp: str) -> str:
UnpatchedSourceDefinition,
UnparsedExposure,
UnparsedModelUpdate,
UnparsedSingularTestUpdate,
)


Expand Down
60 changes: 1 addition & 59 deletions core/dbt/parser/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
ModelNode,
ParsedMacroPatch,
ParsedNodePatch,
ParsedSingularTestPatch,
UnpatchedSourceDefinition,
)
from dbt.contracts.graph.unparsed import (
Expand All @@ -28,7 +27,6 @@
UnparsedMacroUpdate,
UnparsedModelUpdate,
UnparsedNodeUpdate,
UnparsedSingularTestUpdate,
UnparsedSourceDefinition,
)
from dbt.events.types import (
Expand Down Expand Up @@ -209,10 +207,6 @@ def parse_file(self, block: FileBlock, dct: Optional[Dict] = None) -> None:
parser = MacroPatchParser(self, yaml_block, "macros")
parser.parse()

if "data_tests" in dct:
parser = SingularTestPatchParser(self, yaml_block, "data_tests")
parser.parse()

# PatchParser.parse() (but never test_blocks)
if "analyses" in dct:
parser = AnalysisPatchParser(self, yaml_block, "analyses")
Expand Down Expand Up @@ -307,17 +301,14 @@ def _add_yaml_snapshot_nodes_to_manifest(
self.manifest.rebuild_ref_lookup()


Parsed = TypeVar(
"Parsed", UnpatchedSourceDefinition, ParsedNodePatch, ParsedMacroPatch, ParsedSingularTestPatch
)
Parsed = TypeVar("Parsed", UnpatchedSourceDefinition, ParsedNodePatch, ParsedMacroPatch)
NodeTarget = TypeVar("NodeTarget", UnparsedNodeUpdate, UnparsedAnalysisUpdate, UnparsedModelUpdate)
NonSourceTarget = TypeVar(
"NonSourceTarget",
UnparsedNodeUpdate,
UnparsedAnalysisUpdate,
UnparsedMacroUpdate,
UnparsedModelUpdate,
UnparsedSingularTestUpdate,
)


Expand Down Expand Up @@ -1125,55 +1116,6 @@ def _target_type(self) -> Type[UnparsedAnalysisUpdate]:
return UnparsedAnalysisUpdate


class SingularTestPatchParser(PatchParser[UnparsedSingularTestUpdate, ParsedSingularTestPatch]):
def get_block(self, node: UnparsedSingularTestUpdate) -> TargetBlock:
return TargetBlock.from_yaml_block(self.yaml, node)

def _target_type(self) -> Type[UnparsedSingularTestUpdate]:
return UnparsedSingularTestUpdate

def parse_patch(self, block: TargetBlock[UnparsedSingularTestUpdate], refs: ParserRef) -> None:
patch = ParsedSingularTestPatch(
name=block.target.name,
description=block.target.description,
meta=block.target.meta,
docs=block.target.docs,
config=block.target.config,
original_file_path=block.target.original_file_path,
yaml_key=block.target.yaml_key,
package_name=block.target.package_name,
)

assert isinstance(self.yaml.file, SchemaSourceFile)
source_file: SchemaSourceFile = self.yaml.file

unique_id = self.manifest.singular_test_lookup.get_unique_id(
block.name, block.target.package_name
)
if not unique_id:
warn_or_error(
NoNodeForYamlKey(
patch_name=patch.name,
yaml_key=patch.yaml_key,
file_path=source_file.path.original_file_path,
)
)
return

node = self.manifest.nodes.get(unique_id)
assert node is not None

source_file.append_patch(patch.yaml_key, unique_id)
if patch.config:
self.patch_node_config(node, patch)

node.patch_path = patch.file_id
node.description = patch.description
node.created_at = time.time()
node.meta = patch.meta
node.docs = patch.docs


class MacroPatchParser(PatchParser[UnparsedMacroUpdate, ParsedMacroPatch]):
def get_block(self, node: UnparsedMacroUpdate) -> TargetBlock:
return TargetBlock.from_yaml_block(self.yaml, node)
Expand Down
32 changes: 0 additions & 32 deletions tests/functional/data_test_patch/fixtures.py

This file was deleted.

53 changes: 0 additions & 53 deletions tests/functional/data_test_patch/test_singular_test_patch.py

This file was deleted.

7 changes: 3 additions & 4 deletions tests/unit/config/test_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class TestProjectMethods:
def test_all_source_paths(self, project: Project):
assert (
project.all_source_paths.sort()
== ["models", "seeds", "snapshots", "analyses", "macros", "tests"].sort()
== ["models", "seeds", "snapshots", "analyses", "macros"].sort()
)

def test_generic_test_paths(self, project: Project):
Expand Down Expand Up @@ -99,8 +99,7 @@ def test_defaults(self):
self.assertEqual(project.test_paths, ["tests"])
self.assertEqual(project.analysis_paths, ["analyses"])
self.assertEqual(
set(project.docs_paths),
{"models", "seeds", "snapshots", "analyses", "macros", "tests"},
set(project.docs_paths), set(["models", "seeds", "snapshots", "analyses", "macros"])
)
self.assertEqual(project.asset_paths, [])
self.assertEqual(project.target_path, "target")
Expand Down Expand Up @@ -129,7 +128,7 @@ def test_implicit_overrides(self):
)
self.assertEqual(
set(project.docs_paths),
{"other-models", "seeds", "snapshots", "analyses", "macros", "tests"},
set(["other-models", "seeds", "snapshots", "analyses", "macros"]),
)

def test_all_overrides(self):
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/config/test_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def test_from_args(self):
self.assertEqual(config.test_paths, ["tests"])
self.assertEqual(config.analysis_paths, ["analyses"])
self.assertEqual(
set(config.docs_paths), {"models", "seeds", "snapshots", "analyses", "macros", "tests"}
set(config.docs_paths), set(["models", "seeds", "snapshots", "analyses", "macros"])
)
self.assertEqual(config.asset_paths, [])
self.assertEqual(config.target_path, "target")
Expand Down
Loading