From ac06f43d7b98a8dec08bccd6a1131beeb5285bad Mon Sep 17 00:00:00 2001 From: Chenyu Li Date: Tue, 16 Apr 2024 15:26:22 -0700 Subject: [PATCH 1/4] nits --- tests/unit/parser/test_manifest.py | 93 ++++++++++++++ tests/unit/test_parse_manifest.py | 189 ----------------------------- 2 files changed, 93 insertions(+), 189 deletions(-) create mode 100644 tests/unit/parser/test_manifest.py delete mode 100644 tests/unit/test_parse_manifest.py diff --git a/tests/unit/parser/test_manifest.py b/tests/unit/parser/test_manifest.py new file mode 100644 index 00000000000..d7323ca01bc --- /dev/null +++ b/tests/unit/parser/test_manifest.py @@ -0,0 +1,93 @@ +import pytest +from unittest.mock import patch, MagicMock +from argparse import Namespace + + +from dbt.contracts.graph.manifest import Manifest +from dbt.parser.manifest import ManifestLoader +from dbt.config import RuntimeConfig +from dbt.flags import set_from_args + + +@pytest.fixture +def mock_project(): + mock_project = MagicMock(RuntimeConfig) + mock_project.cli_vars = "" + mock_project.args = MagicMock() + mock_project.args.profile = "test" + mock_project.args.target = "test" + mock_project.project_env_vars = {} + mock_project.profile_env_vars = {} + mock_project.project_target_path = "mock_target_path" + mock_project.credentials = MagicMock() + return mock_project + + +class TestPartialParse: + @patch("dbt.parser.manifest.ManifestLoader.build_manifest_state_check") + @patch("dbt.parser.manifest.os.path.exists") + @patch("dbt.parser.manifest.open") + def test_partial_parse_file_path(self, patched_open, patched_os_exist, patched_state_check): + mock_project = MagicMock(RuntimeConfig) + mock_project.project_target_path = "mock_target_path" + patched_os_exist.return_value = True + set_from_args(Namespace(), {}) + ManifestLoader(mock_project, {}) + # by default we use the project_target_path + patched_open.assert_called_with("mock_target_path/partial_parse.msgpack", "rb") + set_from_args(Namespace(partial_parse_file_path="specified_partial_parse_path"), {}) + ManifestLoader(mock_project, {}) + # if specified in flags, we use the specified path + patched_open.assert_called_with("specified_partial_parse_path", "rb") + + def test_profile_hash_change(self, mock_project): + # This test validate that the profile_hash is updated when the connection keys change + profile_hash = "750bc99c1d64ca518536ead26b28465a224be5ffc918bf2a490102faa5a1bcf5" + mock_project.credentials.connection_info.return_value = "test" + set_from_args(Namespace(), {}) + manifest = ManifestLoader(mock_project, {}) + assert manifest.manifest.state_check.profile_hash.checksum == profile_hash + mock_project.credentials.connection_info.return_value = "test1" + manifest = ManifestLoader(mock_project, {}) + assert manifest.manifest.state_check.profile_hash.checksum != profile_hash + + +class TestFailedPartialParse: + @patch("dbt.tracking.track_partial_parser") + @patch("dbt.tracking.active_user") + @patch("dbt.parser.manifest.PartialParsing") + @patch("dbt.parser.manifest.ManifestLoader.read_manifest_for_partial_parse") + @patch("dbt.parser.manifest.ManifestLoader.build_manifest_state_check") + def test_partial_parse_safe_update_project_parser_files_partially( + self, + patched_state_check, + patched_read_manifest_for_partial_parse, + patched_partial_parsing, + patched_active_user, + patched_track_partial_parser, + ): + mock_instance = MagicMock() + mock_instance.skip_parsing.return_value = False + mock_instance.get_parsing_files.side_effect = KeyError("Whoopsie!") + patched_partial_parsing.return_value = mock_instance + + mock_project = MagicMock(RuntimeConfig) + mock_project.project_target_path = "mock_target_path" + + mock_saved_manifest = MagicMock(Manifest) + mock_saved_manifest.files = {} + patched_read_manifest_for_partial_parse.return_value = mock_saved_manifest + + set_from_args(Namespace(), {}) + loader = ManifestLoader(mock_project, {}) + loader.safe_update_project_parser_files_partially({}) + + patched_track_partial_parser.assert_called_once() + exc_info = patched_track_partial_parser.call_args[0][0] + assert "traceback" in exc_info + assert "exception" in exc_info + assert "code" in exc_info + assert "location" in exc_info + assert "full_reparse_reason" in exc_info + assert "KeyError: 'Whoopsie!'" == exc_info["exception"] + assert isinstance(exc_info["code"], str) or isinstance(exc_info["code"], type(None)) diff --git a/tests/unit/test_parse_manifest.py b/tests/unit/test_parse_manifest.py deleted file mode 100644 index 23d6d51446d..00000000000 --- a/tests/unit/test_parse_manifest.py +++ /dev/null @@ -1,189 +0,0 @@ -import unittest -from unittest import mock -from unittest.mock import patch, MagicMock -from argparse import Namespace - -from .utils import config_from_parts_or_dicts, normalize - -from dbt.contracts.files import SourceFile, FileHash, FilePath -from dbt.contracts.graph.manifest import Manifest, ManifestStateCheck -from dbt.parser import manifest -from dbt.parser.manifest import ManifestLoader -from dbt.config import RuntimeConfig -from dbt.flags import set_from_args - - -class MatchingHash(FileHash): - def __init__(self): - return super().__init__("", "") - - def __eq__(self, other): - return True - - -class MismatchedHash(FileHash): - def __init__(self): - return super().__init__("", "") - - def __eq__(self, other): - return False - - -class TestLoader(unittest.TestCase): - def setUp(self): - profile_data = { - "target": "test", - "quoting": {}, - "outputs": { - "test": { - "type": "postgres", - "host": "localhost", - "schema": "analytics", - "user": "test", - "pass": "test", - "dbname": "test", - "port": 1, - } - }, - } - - root_project = { - "name": "root", - "version": "0.1", - "profile": "test", - "project-root": normalize("/usr/src/app"), - "config-version": 2, - } - - self.root_project_config = config_from_parts_or_dicts( - project=root_project, profile=profile_data, cli_vars='{"test_schema_name": "foo"}' - ) - self.parser = mock.MagicMock() - - # Create the Manifest.state_check patcher - @patch("dbt.parser.manifest.ManifestLoader.build_manifest_state_check") - def _mock_state_check(self): - all_projects = self.all_projects - return ManifestStateCheck( - vars_hash=FileHash.from_contents("vars"), - project_hashes={name: FileHash.from_contents(name) for name in all_projects}, - profile_hash=FileHash.from_contents("profile"), - ) - - self.load_state_check = patch( - "dbt.parser.manifest.ManifestLoader.build_manifest_state_check" - ) - self.mock_state_check = self.load_state_check.start() - self.mock_state_check.side_effect = _mock_state_check - - self.loader = manifest.ManifestLoader( - self.root_project_config, {"root": self.root_project_config} - ) - - def _new_manifest(self): - state_check = ManifestStateCheck(MatchingHash(), MatchingHash, []) - manifest = Manifest({}, {}, {}, {}, {}, {}, [], {}) - manifest.state_check = state_check - return manifest - - def _mismatched_file(self, searched, name): - return self._new_file(searched, name, False) - - def _matching_file(self, searched, name): - return self._new_file(searched, name, True) - - def _new_file(self, searched, name, match): - if match: - checksum = MatchingHash() - else: - checksum = MismatchedHash() - path = FilePath( - searched_path=normalize(searched), - relative_path=normalize(name), - project_root=normalize(self.root_project_config.project_root), - ) - return SourceFile(path=path, checksum=checksum) - - -class TestPartialParse(unittest.TestCase): - def setUp(self) -> None: - mock_project = MagicMock(RuntimeConfig) - mock_project.cli_vars = "" - mock_project.args = MagicMock() - mock_project.args.profile = "test" - mock_project.args.target = "test" - mock_project.project_env_vars = {} - mock_project.profile_env_vars = {} - mock_project.project_target_path = "mock_target_path" - mock_project.credentials = MagicMock() - self.mock_project = mock_project - - @patch("dbt.parser.manifest.ManifestLoader.build_manifest_state_check") - @patch("dbt.parser.manifest.os.path.exists") - @patch("dbt.parser.manifest.open") - def test_partial_parse_file_path(self, patched_open, patched_os_exist, patched_state_check): - mock_project = MagicMock(RuntimeConfig) - mock_project.project_target_path = "mock_target_path" - patched_os_exist.return_value = True - set_from_args(Namespace(), {}) - ManifestLoader(mock_project, {}) - # by default we use the project_target_path - patched_open.assert_called_with("mock_target_path/partial_parse.msgpack", "rb") - set_from_args(Namespace(partial_parse_file_path="specified_partial_parse_path"), {}) - ManifestLoader(mock_project, {}) - # if specified in flags, we use the specified path - patched_open.assert_called_with("specified_partial_parse_path", "rb") - - def test_profile_hash_change(self): - # This test validate that the profile_hash is updated when the connection keys change - profile_hash = "750bc99c1d64ca518536ead26b28465a224be5ffc918bf2a490102faa5a1bcf5" - self.mock_project.credentials.connection_info.return_value = "test" - set_from_args(Namespace(), {}) - manifest = ManifestLoader(self.mock_project, {}) - assert manifest.manifest.state_check.profile_hash.checksum == profile_hash - self.mock_project.credentials.connection_info.return_value = "test1" - manifest = ManifestLoader(self.mock_project, {}) - assert manifest.manifest.state_check.profile_hash.checksum != profile_hash - - -class TestFailedPartialParse(unittest.TestCase): - @patch("dbt.tracking.track_partial_parser") - @patch("dbt.tracking.active_user") - @patch("dbt.parser.manifest.PartialParsing") - @patch("dbt.parser.manifest.ManifestLoader.read_manifest_for_partial_parse") - @patch("dbt.parser.manifest.ManifestLoader.build_manifest_state_check") - def test_partial_parse_safe_update_project_parser_files_partially( - self, - patched_state_check, - patched_read_manifest_for_partial_parse, - patched_partial_parsing, - patched_active_user, - patched_track_partial_parser, - ): - mock_instance = MagicMock() - mock_instance.skip_parsing.return_value = False - mock_instance.get_parsing_files.side_effect = KeyError("Whoopsie!") - patched_partial_parsing.return_value = mock_instance - - mock_project = MagicMock(RuntimeConfig) - mock_project.project_target_path = "mock_target_path" - - mock_saved_manifest = MagicMock(Manifest) - mock_saved_manifest.files = {} - patched_read_manifest_for_partial_parse.return_value = mock_saved_manifest - - set_from_args(Namespace(), {}) - loader = ManifestLoader(mock_project, {}) - loader.safe_update_project_parser_files_partially({}) - - patched_track_partial_parser.assert_called_once() - exc_info = patched_track_partial_parser.call_args[0][0] - self.assertIn("traceback", exc_info) - self.assertIn("exception", exc_info) - self.assertIn("code", exc_info) - self.assertIn("location", exc_info) - self.assertIn("full_reparse_reason", exc_info) - self.assertEqual("KeyError: 'Whoopsie!'", exc_info["exception"]) - self.assertTrue( - isinstance(exc_info["code"], str) or isinstance(exc_info["code"], type(None)) - ) From 14493f483c717f422a8d112e451f26f7abdacf7a Mon Sep 17 00:00:00 2001 From: Chenyu Li Date: Tue, 16 Apr 2024 15:44:19 -0700 Subject: [PATCH 2/4] changelog --- .changes/unreleased/Under the Hood-20240416-154405.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Under the Hood-20240416-154405.yaml diff --git a/.changes/unreleased/Under the Hood-20240416-154405.yaml b/.changes/unreleased/Under the Hood-20240416-154405.yaml new file mode 100644 index 00000000000..e8647174456 --- /dev/null +++ b/.changes/unreleased/Under the Hood-20240416-154405.yaml @@ -0,0 +1,6 @@ +kind: Under the Hood +body: Move ManifestLoader test and change it to pytest +time: 2024-04-16T15:44:05.659973-07:00 +custom: + Author: ChenyuLInx + Issue: "9960" From a5ef4c45dd06a73374664340c6181113b608275c Mon Sep 17 00:00:00 2001 From: Chenyu Li Date: Fri, 19 Apr 2024 10:45:08 -0700 Subject: [PATCH 3/4] nits --- .../graph/test_unparsed.py} | 0 tests/unit/parser/test_manifest.py | 2 +- tests/unit/test_parse_manifest.py | 189 ------------------ 3 files changed, 1 insertion(+), 190 deletions(-) rename tests/unit/{test_constraint_parsing.py => contracts/graph/test_unparsed.py} (100%) delete mode 100644 tests/unit/test_parse_manifest.py diff --git a/tests/unit/test_constraint_parsing.py b/tests/unit/contracts/graph/test_unparsed.py similarity index 100% rename from tests/unit/test_constraint_parsing.py rename to tests/unit/contracts/graph/test_unparsed.py diff --git a/tests/unit/parser/test_manifest.py b/tests/unit/parser/test_manifest.py index d7323ca01bc..6a643e444f3 100644 --- a/tests/unit/parser/test_manifest.py +++ b/tests/unit/parser/test_manifest.py @@ -12,7 +12,7 @@ @pytest.fixture def mock_project(): mock_project = MagicMock(RuntimeConfig) - mock_project.cli_vars = "" + mock_project.cli_vars = {} mock_project.args = MagicMock() mock_project.args.profile = "test" mock_project.args.target = "test" diff --git a/tests/unit/test_parse_manifest.py b/tests/unit/test_parse_manifest.py deleted file mode 100644 index 7471b399acd..00000000000 --- a/tests/unit/test_parse_manifest.py +++ /dev/null @@ -1,189 +0,0 @@ -import unittest -from unittest import mock -from unittest.mock import patch, MagicMock -from argparse import Namespace - -from .utils import config_from_parts_or_dicts, normalize - -from dbt.contracts.files import SourceFile, FileHash, FilePath -from dbt.contracts.graph.manifest import Manifest, ManifestStateCheck -from dbt.parser import manifest -from dbt.parser.manifest import ManifestLoader -from dbt.config import RuntimeConfig -from dbt.flags import set_from_args - - -class MatchingHash(FileHash): - def __init__(self): - return super().__init__("", "") - - def __eq__(self, other): - return True - - -class MismatchedHash(FileHash): - def __init__(self): - return super().__init__("", "") - - def __eq__(self, other): - return False - - -class TestLoader(unittest.TestCase): - def setUp(self): - profile_data = { - "target": "test", - "quoting": {}, - "outputs": { - "test": { - "type": "postgres", - "host": "localhost", - "schema": "analytics", - "user": "test", - "pass": "test", - "dbname": "test", - "port": 1, - } - }, - } - - root_project = { - "name": "root", - "version": "0.1", - "profile": "test", - "project-root": normalize("/usr/src/app"), - "config-version": 2, - } - - self.root_project_config = config_from_parts_or_dicts( - project=root_project, profile=profile_data, cli_vars='{"test_schema_name": "foo"}' - ) - self.parser = mock.MagicMock() - - # Create the Manifest.state_check patcher - @patch("dbt.parser.manifest.ManifestLoader.build_manifest_state_check") - def _mock_state_check(self): - all_projects = self.all_projects - return ManifestStateCheck( - vars_hash=FileHash.from_contents("vars"), - project_hashes={name: FileHash.from_contents(name) for name in all_projects}, - profile_hash=FileHash.from_contents("profile"), - ) - - self.load_state_check = patch( - "dbt.parser.manifest.ManifestLoader.build_manifest_state_check" - ) - self.mock_state_check = self.load_state_check.start() - self.mock_state_check.side_effect = _mock_state_check - - self.loader = manifest.ManifestLoader( - self.root_project_config, {"root": self.root_project_config} - ) - - def _new_manifest(self): - state_check = ManifestStateCheck(MatchingHash(), MatchingHash, []) - manifest = Manifest({}, {}, {}, {}, {}, {}, [], {}) - manifest.state_check = state_check - return manifest - - def _mismatched_file(self, searched, name): - return self._new_file(searched, name, False) - - def _matching_file(self, searched, name): - return self._new_file(searched, name, True) - - def _new_file(self, searched, name, match): - if match: - checksum = MatchingHash() - else: - checksum = MismatchedHash() - path = FilePath( - searched_path=normalize(searched), - relative_path=normalize(name), - project_root=normalize(self.root_project_config.project_root), - ) - return SourceFile(path=path, checksum=checksum) - - -class TestPartialParse(unittest.TestCase): - def setUp(self) -> None: - mock_project = MagicMock(RuntimeConfig) - mock_project.cli_vars = {} - mock_project.args = MagicMock() - mock_project.args.profile = "test" - mock_project.args.target = "test" - mock_project.project_env_vars = {} - mock_project.profile_env_vars = {} - mock_project.project_target_path = "mock_target_path" - mock_project.credentials = MagicMock() - self.mock_project = mock_project - - @patch("dbt.parser.manifest.ManifestLoader.build_manifest_state_check") - @patch("dbt.parser.manifest.os.path.exists") - @patch("dbt.parser.manifest.open") - def test_partial_parse_file_path(self, patched_open, patched_os_exist, patched_state_check): - mock_project = MagicMock(RuntimeConfig) - mock_project.project_target_path = "mock_target_path" - patched_os_exist.return_value = True - set_from_args(Namespace(), {}) - ManifestLoader(mock_project, {}) - # by default we use the project_target_path - patched_open.assert_called_with("mock_target_path/partial_parse.msgpack", "rb") - set_from_args(Namespace(partial_parse_file_path="specified_partial_parse_path"), {}) - ManifestLoader(mock_project, {}) - # if specified in flags, we use the specified path - patched_open.assert_called_with("specified_partial_parse_path", "rb") - - def test_profile_hash_change(self): - # This test validate that the profile_hash is updated when the connection keys change - profile_hash = "750bc99c1d64ca518536ead26b28465a224be5ffc918bf2a490102faa5a1bcf5" - self.mock_project.credentials.connection_info.return_value = "test" - set_from_args(Namespace(), {}) - manifest = ManifestLoader(self.mock_project, {}) - assert manifest.manifest.state_check.profile_hash.checksum == profile_hash - self.mock_project.credentials.connection_info.return_value = "test1" - manifest = ManifestLoader(self.mock_project, {}) - assert manifest.manifest.state_check.profile_hash.checksum != profile_hash - - -class TestFailedPartialParse(unittest.TestCase): - @patch("dbt.tracking.track_partial_parser") - @patch("dbt.tracking.active_user") - @patch("dbt.parser.manifest.PartialParsing") - @patch("dbt.parser.manifest.ManifestLoader.read_manifest_for_partial_parse") - @patch("dbt.parser.manifest.ManifestLoader.build_manifest_state_check") - def test_partial_parse_safe_update_project_parser_files_partially( - self, - patched_state_check, - patched_read_manifest_for_partial_parse, - patched_partial_parsing, - patched_active_user, - patched_track_partial_parser, - ): - mock_instance = MagicMock() - mock_instance.skip_parsing.return_value = False - mock_instance.get_parsing_files.side_effect = KeyError("Whoopsie!") - patched_partial_parsing.return_value = mock_instance - - mock_project = MagicMock(RuntimeConfig) - mock_project.project_target_path = "mock_target_path" - - mock_saved_manifest = MagicMock(Manifest) - mock_saved_manifest.files = {} - patched_read_manifest_for_partial_parse.return_value = mock_saved_manifest - - set_from_args(Namespace(), {}) - loader = ManifestLoader(mock_project, {}) - loader.safe_update_project_parser_files_partially({}) - - patched_track_partial_parser.assert_called_once() - exc_info = patched_track_partial_parser.call_args[0][0] - self.assertIn("traceback", exc_info) - self.assertIn("exception", exc_info) - self.assertIn("code", exc_info) - self.assertIn("location", exc_info) - self.assertIn("full_reparse_reason", exc_info) - self.assertEqual("KeyError: 'Whoopsie!'", exc_info["exception"]) - self.assertTrue( - isinstance(exc_info["code"], str) or isinstance(exc_info["code"], type(None)) - ) From bceae818bb22916e02642ec22412ceb6a50d25ce Mon Sep 17 00:00:00 2001 From: Chenyu Li Date: Fri, 19 Apr 2024 10:45:55 -0700 Subject: [PATCH 4/4] nits --- .changes/unreleased/Under the Hood-20240416-154405.yaml | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 .changes/unreleased/Under the Hood-20240416-154405.yaml diff --git a/.changes/unreleased/Under the Hood-20240416-154405.yaml b/.changes/unreleased/Under the Hood-20240416-154405.yaml deleted file mode 100644 index e8647174456..00000000000 --- a/.changes/unreleased/Under the Hood-20240416-154405.yaml +++ /dev/null @@ -1,6 +0,0 @@ -kind: Under the Hood -body: Move ManifestLoader test and change it to pytest -time: 2024-04-16T15:44:05.659973-07:00 -custom: - Author: ChenyuLInx - Issue: "9960"