From 7fdcc3f5596f5f30a1d51a2ff69551a7874b0daa Mon Sep 17 00:00:00 2001 From: aditya-balachander Date: Fri, 12 Dec 2025 12:47:06 +0530 Subject: [PATCH 1/3] Raise validation errors for missing schema and permissions --- cumulusci/tasks/bulkdata/mapping_parser.py | 20 +++++----- .../bulkdata/tests/test_mapping_parser.py | 40 +++++++++---------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/cumulusci/tasks/bulkdata/mapping_parser.py b/cumulusci/tasks/bulkdata/mapping_parser.py index 7152ae94b8..4278dda6bd 100644 --- a/cumulusci/tasks/bulkdata/mapping_parser.py +++ b/cumulusci/tasks/bulkdata/mapping_parser.py @@ -444,9 +444,9 @@ def replace_if_necessary(dct, name, replacement): except KeyError: message = f"Field {self.sf_object}.{f} does not exist or is not visible to the current user." if validation_result: - validation_result.add_warning(message) + validation_result.add_error(message) else: - logger.warning(message) + logger.error(message) else: del field_dict[f] field_dict[new_name] = entry @@ -463,9 +463,9 @@ def replace_if_necessary(dct, name, replacement): if f not in describe: message = f"Field {self.sf_object}.{f} does not exist or is not visible to the current user." if validation_result: - validation_result.add_warning(message) + validation_result.add_error(message) else: - logger.warning(message) + logger.error(message) error_in_f = True elif not self._check_field_permission( describe, @@ -480,9 +480,9 @@ def replace_if_necessary(dct, name, replacement): + f"{relevant_permissions} for this operation." ) if validation_result: - validation_result.add_warning(message) + validation_result.add_error(message) else: - logger.warning(message) + logger.error(message) error_in_f = True if error_in_f: @@ -514,9 +514,9 @@ def _validate_sobject( except KeyError: message = f"sObject {self.sf_object} does not exist or is not visible to the current user." if validation_result: - validation_result.add_warning(message) + validation_result.add_error(message) else: - logger.warning(message) + logger.error(message) return False # Validate our access to this sObject. @@ -525,9 +525,9 @@ def _validate_sobject( ): message = f"sObject {self.sf_object} does not have the correct permissions for {data_operation_type}." if validation_result: - validation_result.add_warning(message) + validation_result.add_error(message) else: - logger.warning(message) + logger.error(message) return False return True diff --git a/cumulusci/tasks/bulkdata/tests/test_mapping_parser.py b/cumulusci/tasks/bulkdata/tests/test_mapping_parser.py index 045ebe0260..969b910fdb 100644 --- a/cumulusci/tasks/bulkdata/tests/test_mapping_parser.py +++ b/cumulusci/tasks/bulkdata/tests/test_mapping_parser.py @@ -1868,8 +1868,9 @@ def test_validate_only_collects_missing_field_errors(self): assert result is not None assert isinstance(result, ValidationResult) - # Should have warnings about missing field - assert any("Nonsense__c" in warning for warning in result.warnings) + assert result.has_errors() + # Should have errors about missing field + assert any("Nonsense__c" in error for error in result.errors) @responses.activate def test_validate_only_collects_missing_required_field_errors(self): @@ -1930,8 +1931,9 @@ def test_validate_only_early_return_on_sobject_error(self): assert result is not None assert isinstance(result, ValidationResult) - # Should have warning about missing object - assert any("InvalidObject__c" in warning for warning in result.warnings) + assert result.has_errors() + # Should have error about missing object + assert any("InvalidObject__c" in error for error in result.errors) @responses.activate def test_validate_only_collects_lookup_errors(self): @@ -2068,7 +2070,7 @@ def test_check_required_without_validation_result_logs(self, caplog): assert "Name" in caplog.text def test_validate_sobject_with_validation_result(self): - """Test _validate_sobject adds warnings to ValidationResult""" + """Test _validate_sobject adds errors to ValidationResult""" from cumulusci.tasks.bulkdata.mapping_parser import ValidationResult ms = MappingStep( @@ -2087,13 +2089,11 @@ def test_validate_sobject_with_validation_result(self): ) assert not result - assert len(validation_result.warnings) > 0 - assert any( - "InvalidObject__c" in warning for warning in validation_result.warnings - ) + assert validation_result.has_errors() + assert any("InvalidObject__c" in error for error in validation_result.errors) def test_validate_field_dict_with_validation_result(self): - """Test _validate_field_dict adds warnings to ValidationResult""" + """Test _validate_field_dict adds errors to ValidationResult""" from cumulusci.tasks.bulkdata.mapping_parser import ValidationResult ms = MappingStep( @@ -2114,10 +2114,8 @@ def test_validate_field_dict_with_validation_result(self): ) assert not result - assert len(validation_result.warnings) > 0 - assert any( - "NonexistentField__c" in warning for warning in validation_result.warnings - ) + assert validation_result.has_errors() + assert any("NonexistentField__c" in error for error in validation_result.errors) def test_infer_and_validate_lookups_with_validation_result(self): """Test _infer_and_validate_lookups adds errors to ValidationResult""" @@ -2235,10 +2233,11 @@ def test_validate_field_dict_permission_error_with_validation_result(self): ) assert not result - # Should have warning about incorrect permissions + assert validation_result.has_errors() + # Should have error about incorrect permissions assert any( - "does not have the correct permissions" in warning - for warning in validation_result.warnings + "does not have the correct permissions" in error + for error in validation_result.errors ) def test_validate_sobject_permission_error_with_validation_result(self): @@ -2261,10 +2260,11 @@ def test_validate_sobject_permission_error_with_validation_result(self): ) assert not result - # Should have warning about incorrect permissions + assert validation_result.has_errors() + # Should have error about incorrect permissions assert any( - "does not have the correct permissions" in warning - for warning in validation_result.warnings + "does not have the correct permissions" in error + for error in validation_result.errors ) def test_infer_and_validate_lookups_invalid_reference_with_validation_result(self): From 9e440290963424b2ab3c4f76f563ecb8f3d47602 Mon Sep 17 00:00:00 2001 From: aditya-balachander Date: Fri, 19 Dec 2025 10:27:34 +0530 Subject: [PATCH 2/3] Add strict-mode to snowfakery task --- .../tasks/bulkdata/generate_from_yaml.py | 4 + cumulusci/tasks/bulkdata/snowfakery.py | 7 +- .../test_generate_from_snowfakery_task.py | 96 +++++++++++++++++++ .../tasks/bulkdata/tests/test_snowfakery.py | 53 ++++++++++ pyproject.toml | 2 +- uv.lock | 8 +- 6 files changed, 162 insertions(+), 8 deletions(-) diff --git a/cumulusci/tasks/bulkdata/generate_from_yaml.py b/cumulusci/tasks/bulkdata/generate_from_yaml.py index b86a776878..fcd0c818f5 100644 --- a/cumulusci/tasks/bulkdata/generate_from_yaml.py +++ b/cumulusci/tasks/bulkdata/generate_from_yaml.py @@ -95,6 +95,9 @@ def _init_options(self, kwargs): self.working_directory = self.options.get("working_directory") loading_rules = process_list_arg(self.options.get("loading_rules")) or [] self.loading_rules = [Path(path) for path in loading_rules if path] + # These flags may be provided internally by callers (e.g., Snowfakery task) + self.strict_mode = bool(self.options.get("strict_mode", False)) + self.validate_only = bool(self.options.get("validate_only", False)) def _generate_data(self, db_url, mapping_file_path, num_records, current_batch_num): """Generate all of the data""" @@ -171,6 +174,7 @@ def generate_data(self, dburl, num_records, current_batch_num): "project_config": self.project_config, **self.plugin_options, }, + strict_mode=self.strict_mode or self.validate_only, ) if ( diff --git a/cumulusci/tasks/bulkdata/snowfakery.py b/cumulusci/tasks/bulkdata/snowfakery.py index 340e2d76ac..0f960bd29e 100644 --- a/cumulusci/tasks/bulkdata/snowfakery.py +++ b/cumulusci/tasks/bulkdata/snowfakery.py @@ -141,9 +141,12 @@ class Snowfakery(BaseSalesforceApiTask): "Defaults to False." }, "validate_only": { - "description": "Boolean: if True, only validate the generated mapping against the org schema without loading data. " + "description": "Boolean: if True, validates snowfakery recipe and generated mapping against the org schema without loading data. " "Defaults to False." }, + "strict_mode": { + "description": "Boolean: If True, validates the Snowfakery recipe and generated mapping against the org schema (strict mode) and then proceeds with the run", + }, } def _validate_options(self): @@ -165,6 +168,7 @@ def _validate_options(self): self.options.get("drop_missing_schema", False) ) self.validate_only = process_bool_arg(self.options.get("validate_only", False)) + self.strict_mode = process_bool_arg(self.options.get("strict_mode", False)) loading_rules = process_list_arg(self.options.get("loading_rules")) or [] self.loading_rules = [Path(path) for path in loading_rules if path] @@ -614,6 +618,7 @@ def _run_generate_and_load_subtask( "ignore_row_errors": self.ignore_row_errors, "drop_missing_schema": self.drop_missing_schema, "validate_only": validate_only, + "strict_mode": self.strict_mode, } subtask_config = TaskConfig({"options": options}) subtask = GenerateAndLoadDataFromYaml( diff --git a/cumulusci/tasks/bulkdata/tests/test_generate_from_snowfakery_task.py b/cumulusci/tasks/bulkdata/tests/test_generate_from_snowfakery_task.py index 83bf6ef7ef..87845ebc8b 100644 --- a/cumulusci/tasks/bulkdata/tests/test_generate_from_snowfakery_task.py +++ b/cumulusci/tasks/bulkdata/tests/test_generate_from_snowfakery_task.py @@ -206,6 +206,102 @@ def test_exception_handled_cleanly(self, generate_data): assert "Foo" in str(e.value) assert len(generate_data.mock_calls) == 1 + @mock.patch("cumulusci.tasks.bulkdata.generate_from_yaml.generate_data") + def test_validate_only_forces_strict_mode(self, generate_data): + with temp_sqlite_database_url() as database_url: + task = _make_task( + GenerateDataFromYaml, + { + "options": { + "generator_yaml": simple_yaml, + "database_url": database_url, + "validate_only": True, + } + }, + ) + task() + + assert len(generate_data.mock_calls) == 1 + _, kwargs = generate_data.call_args + assert kwargs["strict_mode"] is True + + @mock.patch("cumulusci.tasks.bulkdata.generate_from_yaml.generate_data") + def test_strict_mode_only(self, generate_data): + with temp_sqlite_database_url() as database_url: + task = _make_task( + GenerateDataFromYaml, + { + "options": { + "generator_yaml": simple_yaml, + "database_url": database_url, + "strict_mode": True, + } + }, + ) + task() + + assert len(generate_data.mock_calls) == 1 + _, kwargs = generate_data.call_args + assert kwargs["strict_mode"] is True + + @mock.patch("cumulusci.tasks.bulkdata.generate_from_yaml.generate_data") + def test_defaults_no_strict_no_validate(self, generate_data): + with temp_sqlite_database_url() as database_url: + task = _make_task( + GenerateDataFromYaml, + { + "options": { + "generator_yaml": simple_yaml, + "database_url": database_url, + } + }, + ) + task() + + assert len(generate_data.mock_calls) == 1 + _, kwargs = generate_data.call_args + assert kwargs["strict_mode"] is False + + def test_validate_only_runs_and_creates_mapping(self): + with temporary_file_path("mapping.yml") as mapping_path: + with temp_sqlite_database_url() as database_url: + task = _make_task( + GenerateDataFromYaml, + { + "options": { + "generator_yaml": simple_yaml, + "database_url": database_url, + "generate_mapping_file": mapping_path, + "validate_only": True, + } + }, + ) + task() + + assert mapping_path.exists() + mapping = yaml.safe_load(open(mapping_path)) + assert mapping # ensure something was written + + def test_strict_mode_runs_and_creates_mapping(self): + with temporary_file_path("mapping.yml") as mapping_path: + with temp_sqlite_database_url() as database_url: + task = _make_task( + GenerateDataFromYaml, + { + "options": { + "generator_yaml": simple_yaml, + "database_url": database_url, + "generate_mapping_file": mapping_path, + "strict_mode": True, + } + }, + ) + task() + + assert mapping_path.exists() + mapping = yaml.safe_load(open(mapping_path)) + assert mapping # ensure something was written + @mock.patch( "cumulusci.tasks.bulkdata.generate_and_load_data_from_yaml.GenerateAndLoadDataFromYaml._dataload" ) diff --git a/cumulusci/tasks/bulkdata/tests/test_snowfakery.py b/cumulusci/tasks/bulkdata/tests/test_snowfakery.py index b30704c518..7b30cbc6fc 100644 --- a/cumulusci/tasks/bulkdata/tests/test_snowfakery.py +++ b/cumulusci/tasks/bulkdata/tests/test_snowfakery.py @@ -237,6 +237,59 @@ def _run_snowfakery_and_inspect_mapping(**options): return _run_snowfakery_and_inspect_mapping +@mock.patch("cumulusci.tasks.bulkdata.snowfakery.GenerateAndLoadDataFromYaml") +def test_snowfakery_validate_only_passes_flags(mock_subtask_cls, snowfakery): + mock_subtask = mock.Mock() + mock_subtask.__call__ = mock.Mock(return_value=None) + mock_subtask.return_values = {"validation_result": "ok"} + mock_subtask_cls.return_value = mock_subtask + + task = snowfakery( + recipe=str(simple_salesforce_yaml), + validate_only=True, + ) + + with TemporaryDirectory() as tmpdir: + task._run_generate_and_load_subtask( + Path(tmpdir), + DummyOrgConfig({}, "test"), + options={}, + validate_only=True, + ) + + # task_config passed into subtask should carry validate_only=True and strict_mode flag + call_kwargs = mock_subtask_cls.call_args.kwargs + task_config = call_kwargs["task_config"] + assert task_config.options["validate_only"] is True + assert task_config.options["strict_mode"] is False + + +@mock.patch("cumulusci.tasks.bulkdata.snowfakery.GenerateAndLoadDataFromYaml") +def test_snowfakery_strict_mode_passes_flags(mock_subtask_cls, snowfakery): + mock_subtask = mock.Mock() + mock_subtask.__call__ = mock.Mock(return_value=None) + mock_subtask.return_values = {"validation_result": "ok"} + mock_subtask_cls.return_value = mock_subtask + + task = snowfakery( + recipe=str(simple_salesforce_yaml), + strict_mode=True, + ) + + with TemporaryDirectory() as tmpdir: + task._run_generate_and_load_subtask( + Path(tmpdir), + DummyOrgConfig({}, "test"), + options={}, + validate_only=False, + ) + + call_kwargs = mock_subtask_cls.call_args.kwargs + task_config = call_kwargs["task_config"] + assert task_config.options["validate_only"] is False + assert task_config.options["strict_mode"] is True + + def get_mapping_from_snowfakery_task_results(results: SnowfakeryTaskResults): """Find the shared mapping file and return it.""" template_dir = SnowfakeryWorkingDirectory(results.working_dir / "template_1/") diff --git a/pyproject.toml b/pyproject.toml index 66cdbf7053..90cc90735a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -53,7 +53,7 @@ dependencies = [ "sarge", "selenium<4", "simple-salesforce==1.11.4", - "snowfakery>=4.1.0", + "snowfakery @ git+https://github.com/SFDO-Tooling/Snowfakery.git@W-19976090/standard-function-validation", "xmltodict", "docutils<=0.21.2", ] diff --git a/uv.lock b/uv.lock index 071af4e7ba..5adcfc690d 100644 --- a/uv.lock +++ b/uv.lock @@ -505,7 +505,7 @@ requires-dist = [ { name = "scikit-learn", marker = "extra == 'select'" }, { name = "selenium", specifier = "<4" }, { name = "simple-salesforce", specifier = "==1.11.4" }, - { name = "snowfakery", specifier = ">=4.1.0" }, + { name = "snowfakery", git = "https://github.com/SFDO-Tooling/Snowfakery.git?rev=W-19976090%2Fstandard-function-validation" }, { name = "sqlalchemy", specifier = "<2" }, { name = "xmltodict" }, ] @@ -2106,7 +2106,7 @@ wheels = [ [[package]] name = "snowfakery" version = "4.1.0" -source = { registry = "https://pypi.org/simple" } +source = { git = "https://github.com/SFDO-Tooling/Snowfakery.git?rev=W-19976090%2Fstandard-function-validation#0b5a9b68ca051649b157dcd01d12cdb2bfe34016" } dependencies = [ { name = "click" }, { name = "faker" }, @@ -2122,10 +2122,6 @@ dependencies = [ { name = "setuptools" }, { name = "sqlalchemy" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/bf/6d/8e13daa8b9ebc5f6b24351ab0bf3a25be59bc24d68b9f7636e7ce779957d/snowfakery-4.1.0.tar.gz", hash = "sha256:d589c14ab7673649023138b6ef468a51e8bc1c390db624be115d1adbfe5c1d36", size = 76085, upload-time = "2025-10-09T05:51:05.144Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/71/87/1f9b9dc9ee25c32aff7f90187d64c727baa01ca1a8b4d1a43bb982110929/snowfakery-4.1.0-py3-none-any.whl", hash = "sha256:c13453ae5240bbc3d0ccf6049d3c9f1989d0c803672e88f7cff25419828c6ce0", size = 100778, upload-time = "2025-10-09T05:51:06.514Z" }, -] [[package]] name = "soupsieve" From 1a77cbac55b597f4a12fb6af4616481efa865cb2 Mon Sep 17 00:00:00 2001 From: aditya-balachander Date: Fri, 19 Dec 2025 14:50:36 +0530 Subject: [PATCH 3/3] Upgrade snowfakery version --- pyproject.toml | 2 +- uv.lock | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 90cc90735a..a25a1883cd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -53,7 +53,7 @@ dependencies = [ "sarge", "selenium<4", "simple-salesforce==1.11.4", - "snowfakery @ git+https://github.com/SFDO-Tooling/Snowfakery.git@W-19976090/standard-function-validation", + "snowfakery>=4.2.0", "xmltodict", "docutils<=0.21.2", ] diff --git a/uv.lock b/uv.lock index 5adcfc690d..480331b983 100644 --- a/uv.lock +++ b/uv.lock @@ -505,7 +505,7 @@ requires-dist = [ { name = "scikit-learn", marker = "extra == 'select'" }, { name = "selenium", specifier = "<4" }, { name = "simple-salesforce", specifier = "==1.11.4" }, - { name = "snowfakery", git = "https://github.com/SFDO-Tooling/Snowfakery.git?rev=W-19976090%2Fstandard-function-validation" }, + { name = "snowfakery", specifier = ">=4.2.0" }, { name = "sqlalchemy", specifier = "<2" }, { name = "xmltodict" }, ] @@ -2105,8 +2105,8 @@ wheels = [ [[package]] name = "snowfakery" -version = "4.1.0" -source = { git = "https://github.com/SFDO-Tooling/Snowfakery.git?rev=W-19976090%2Fstandard-function-validation#0b5a9b68ca051649b157dcd01d12cdb2bfe34016" } +version = "4.2.0" +source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "click" }, { name = "faker" }, @@ -2122,6 +2122,10 @@ dependencies = [ { name = "setuptools" }, { name = "sqlalchemy" }, ] +sdist = { url = "https://files.pythonhosted.org/packages/ea/48/b155854cd42d4bf345873fc11bdeb3257a505914de088dc8a0ec90e8753e/snowfakery-4.2.0.tar.gz", hash = "sha256:930df06131749d033559e5edb4a60daa747beacafa8a36b07093d23da7095908", size = 110275, upload-time = "2025-12-19T09:17:56.172Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/fe/00/dca6dd168eeb2783e0e667923155cee1695b594bdcb22c27214379214728/snowfakery-4.2.0-py3-none-any.whl", hash = "sha256:e5df8361c86bc70741fdb8d31d2e420d0bb9e0c804097874756911ec6d2ee49b", size = 138789, upload-time = "2025-12-19T09:17:54.94Z" }, +] [[package]] name = "soupsieve"