From c65382a1139d6e0e2d378513195d9455010f9198 Mon Sep 17 00:00:00 2001 From: James Estevez Date: Mon, 28 Nov 2022 17:17:01 -0800 Subject: [PATCH 1/4] Add 3.11 to feature test workflows Includes 3.11 in our feature tests. Adds the continue-on-error key to bypass Windows failures until lxml/lxml 356 is resolved. --- .github/workflows/feature_test.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/feature_test.yml b/.github/workflows/feature_test.yml index 44fba3ce9c..3526ec78fa 100644 --- a/.github/workflows/feature_test.yml +++ b/.github/workflows/feature_test.yml @@ -41,11 +41,13 @@ jobs: unit_tests: name: "Unit tests: ${{ matrix.os }}-${{ matrix.python-version }}" runs-on: ${{ matrix.os }} + # FIXME: Remove when https://github.com/lxml/lxml/pull/356 is released upstream + continue-on-error: ${{ matrix.os == 'sfdc-windows-latest' && matrix.python-version == '3.11' }} strategy: fail-fast: false matrix: os: [macos-latest, sfdc-ubuntu-latest, sfdc-windows-latest] - python-version: ["3.8", "3.9", "3.10"] + python-version: ["3.8", "3.9", "3.10", "3.11"] steps: - uses: actions/checkout@v2 - name: Set up Python From 9648f87e28243db8f56bff834694850c28cf2b5e Mon Sep 17 00:00:00 2001 From: James Estevez Date: Tue, 4 Oct 2022 14:03:38 -0700 Subject: [PATCH 2/4] Update mock.patch namespaces for 3.11 Because class paths are used in Task definitions we go into import contortions to preserve them[^1]. 3.11 includes an update that replaces `unittest.mock._importer` with `pkgutil.resolve_name`, breaking mock.patch in several tests. For example: ```python-console Python 3.11.0 (main, Nov 29 2022, 08:46:07) [Clang 14.0.0 (clang-1400.0.29.202)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> clspth = "cumulusci.tasks.salesforce.Deploy.__call__" >>> import pkgutil >>> getter = lambda: pkgutil.resolve_name(clspth) >>> target = getter() Traceback (most recent call last): File "", line 1, in File "", line 1, in File "/Users/jestevez/.pyenv/versions/3.11.0/lib/python3.11/pkgutil.py", line 715, in resolve_name result = getattr(result, p) ^^^^^^^^^^^^^^^^^^ AttributeError: module 'cumulusci.tasks.salesforce.Deploy' has no attribute '__call__'. Did you mean: '__file__'? >>> # However >>> from cumulusci.tasks.salesforce import Deploy >>> getattr(Deploy, '__call__') ``` Updating our patches to point at the right namespace works for most of the mocks on 3.8+. [^1]: Read "contortions" as "I don't understand them." --- cumulusci/cli/tests/test_cci.py | 12 +++++++---- cumulusci/core/config/tests/test_config.py | 12 +++++------ .../config/tests/test_config_expensive.py | 20 ++++++++++++++----- cumulusci/tasks/apex/tests/test_apex_tasks.py | 6 +++--- .../tasks/salesforce/tests/test_base_tasks.py | 4 +--- .../tests/test_network_member_group.py | 7 ++----- cumulusci/tasks/salesforce/tests/util.py | 2 +- cumulusci/tasks/tests/test_salesforce.py | 2 +- cumulusci/tests/util.py | 2 +- 9 files changed, 38 insertions(+), 29 deletions(-) diff --git a/cumulusci/cli/tests/test_cci.py b/cumulusci/cli/tests/test_cci.py index 1fae62beb9..ccb06adaea 100644 --- a/cumulusci/cli/tests/test_cci.py +++ b/cumulusci/cli/tests/test_cci.py @@ -2,6 +2,7 @@ import io import os import shutil +import sys import tempfile from pathlib import Path from unittest import mock @@ -262,13 +263,16 @@ def test_cci_org_default__no_orgname( assert "There is no default org" in stdout.getvalue() +DEPLOY_CLASS_PATH = f"cumulusci.tasks.salesforce.Deploy{'.Deploy' if sys.version_info >= (3, 11) else ''}" + + @mock.patch("cumulusci.cli.cci.init_logger", mock.Mock()) @mock.patch("cumulusci.cli.cci.tee_stdout_stderr", mock.MagicMock()) -@mock.patch("cumulusci.tasks.salesforce.Deploy.__call__", mock.Mock()) +@mock.patch(f"{DEPLOY_CLASS_PATH}.__call__", mock.Mock()) @mock.patch("sys.exit", mock.Mock()) @mock.patch("cumulusci.cli.cci.get_tempfile_logger") @mock.patch("cumulusci.cli.cci.CliRuntime") -@mock.patch("cumulusci.tasks.salesforce.Deploy.__init__") +@mock.patch(f"{DEPLOY_CLASS_PATH}.__init__") def test_cci_run_task_options__with_dash( Deploy, CliRuntime, @@ -296,11 +300,11 @@ def test_cci_run_task_options__with_dash( @mock.patch("cumulusci.cli.cci.init_logger", mock.Mock()) @mock.patch("cumulusci.cli.cci.tee_stdout_stderr", mock.MagicMock()) -@mock.patch("cumulusci.tasks.salesforce.Deploy.__call__", mock.Mock()) +@mock.patch(f"{DEPLOY_CLASS_PATH}.__call__", mock.Mock()) @mock.patch("sys.exit", mock.Mock()) @mock.patch("cumulusci.cli.cci.get_tempfile_logger") @mock.patch("cumulusci.cli.cci.CliRuntime") -@mock.patch("cumulusci.tasks.salesforce.Deploy.__init__") +@mock.patch(f"{DEPLOY_CLASS_PATH}.__init__") def test_cci_run_task_options__old_style_with_dash( Deploy, CliRuntime, diff --git a/cumulusci/core/config/tests/test_config.py b/cumulusci/core/config/tests/test_config.py index cd1ebfb0fe..e833b962e4 100644 --- a/cumulusci/core/config/tests/test_config.py +++ b/cumulusci/core/config/tests/test_config.py @@ -854,7 +854,7 @@ def test_suggested_name(self): class TestOrgConfig: - @mock.patch("cumulusci.core.config.OrgConfig.OAuth2Client") + @mock.patch("cumulusci.core.config.org_config.OrgConfig.OAuth2Client") def test_refresh_oauth_token(self, OAuth2Client): config = OrgConfig( { @@ -877,7 +877,7 @@ def test_refresh_oauth_token(self, OAuth2Client): assert client_config.client_id == DEFAULT_CONNECTED_APP.client_id refresh_token.assert_called_once_with(mock.sentinel.refresh_token) - @mock.patch("cumulusci.core.config.OrgConfig.OAuth2Client") + @mock.patch("cumulusci.core.config.org_config.OrgConfig.OAuth2Client") def test_refresh_oauth_token__other_connected_app(self, OAuth2Client): config = OrgConfig( { @@ -1143,7 +1143,7 @@ def test_get_community_info__fetch_if_not_in_cache(self): info = config.get_community_info("Kōkua") assert info["name"] == "Kōkua" - @mock.patch("cumulusci.core.config.OrgConfig._fetch_community_info") + @mock.patch("cumulusci.core.config.org_config.OrgConfig._fetch_community_info") def test_community_info_force_refresh(self, mock_fetch): """Verify that the force_refresh parameter has an effect""" mock_fetch.return_value = {"Kōkua": {"name": "Kōkua"}} @@ -1159,7 +1159,7 @@ def test_community_info_force_refresh(self, mock_fetch): config.get_community_info("Kōkua", force_refresh=True) mock_fetch.assert_called() - @mock.patch("cumulusci.core.config.OrgConfig._fetch_community_info") + @mock.patch("cumulusci.core.config.org_config.OrgConfig._fetch_community_info") def test_community_info_exception(self, mock_fetch): """Verify an exception is thrown when the community doesn't exist""" config = OrgConfig({}, "test") @@ -1259,7 +1259,7 @@ def test_community_info_exception(self, mock_fetch): SalesforceError(None, None, None, None), ] - @mock.patch("cumulusci.core.config.OrgConfig.salesforce_client") + @mock.patch("cumulusci.core.config.org_config.OrgConfig.salesforce_client") def test_installed_packages(self, sf): config = OrgConfig({}, "test") sf.restful.side_effect = self.MOCK_TOOLING_PACKAGE_RESULTS @@ -1300,7 +1300,7 @@ def test_installed_packages(self, sf): assert config.installed_packages == expected sf.restful.assert_called() - @mock.patch("cumulusci.core.config.OrgConfig.salesforce_client") + @mock.patch("cumulusci.core.config.org_config.OrgConfig.salesforce_client") def test_has_minimum_package_version(self, sf): config = OrgConfig({}, "test") sf.restful.side_effect = self.MOCK_TOOLING_PACKAGE_RESULTS diff --git a/cumulusci/core/config/tests/test_config_expensive.py b/cumulusci/core/config/tests/test_config_expensive.py index 0e558ebcbd..5e1cd23b8c 100644 --- a/cumulusci/core/config/tests/test_config_expensive.py +++ b/cumulusci/core/config/tests/test_config_expensive.py @@ -370,7 +370,9 @@ def test_get_access_token(self, Command): sfdx = mock.Mock(return_value=sfdx_response) config = ScratchOrgConfig({}, "test") - with mock.patch("cumulusci.core.config.OrgConfig.salesforce_client", sf): + with mock.patch( + "cumulusci.core.config.org_config.OrgConfig.salesforce_client", sf + ): with mock.patch("cumulusci.core.config.sfdx_org_config.sfdx", sfdx): access_token = config.get_access_token(alias="dadvisor") sfdx.assert_called_once_with( @@ -392,7 +394,9 @@ def test_get_access_token__unknown_user(self, Command): config = ScratchOrgConfig({}, "test") - with mock.patch("cumulusci.core.config.OrgConfig.salesforce_client", sf): + with mock.patch( + "cumulusci.core.config.org_config.OrgConfig.salesforce_client", sf + ): with pytest.raises( SfdxOrgException, match="Couldn't find a username for the specified user", @@ -410,7 +414,9 @@ def test_get_access_token__multiple_users(self, Command): config = ScratchOrgConfig({}, "test") - with mock.patch("cumulusci.core.config.OrgConfig.salesforce_client", sf): + with mock.patch( + "cumulusci.core.config.org_config.OrgConfig.salesforce_client", sf + ): with pytest.raises( SfdxOrgException, match="More than one user matched the search critiera.", @@ -426,7 +432,9 @@ def test_get_access_token__no_access_token(self, Command): sfdx = mock.Mock(return_value=sfdx_response) config = ScratchOrgConfig({}, "test") - with mock.patch("cumulusci.core.config.OrgConfig.salesforce_client", sf): + with mock.patch( + "cumulusci.core.config.org_config.OrgConfig.salesforce_client", sf + ): with mock.patch("cumulusci.core.config.sfdx_org_config.sfdx", sfdx): exception = ( "Unable to find access token for whatever@example.com\nblah blah..." @@ -463,7 +471,9 @@ def test_user_id_from_org(self, Command): "instance_url": "test_instance", "access_token": "token", } - with mock.patch("cumulusci.core.config.OrgConfig.salesforce_client", sf): + with mock.patch( + "cumulusci.core.config.org_config.OrgConfig.salesforce_client", sf + ): assert config.user_id == "test" def test_username_from_sfdx_info(self, Command): diff --git a/cumulusci/tasks/apex/tests/test_apex_tasks.py b/cumulusci/tasks/apex/tests/test_apex_tasks.py index 23b21a03fb..e06393f25c 100644 --- a/cumulusci/tasks/apex/tests/test_apex_tasks.py +++ b/cumulusci/tasks/apex/tests/test_apex_tasks.py @@ -34,7 +34,7 @@ @patch( - "cumulusci.tasks.salesforce.BaseSalesforceTask._update_credentials", + "cumulusci.core.tasks.BaseSalesforceTask._update_credentials", MagicMock(return_value=None), ) class TestRunApexTests(MockLoggerMixin): @@ -757,7 +757,7 @@ def test_run_task__no_tests(self): @patch( - "cumulusci.tasks.salesforce.BaseSalesforceTask._update_credentials", + "cumulusci.core.tasks.BaseSalesforceTask._update_credentials", MagicMock(return_value=None), ) class TestAnonymousApexTask: @@ -934,7 +934,7 @@ def test_run_anonymous_apex__gack(self): @patch( - "cumulusci.tasks.salesforce.BaseSalesforceTask._update_credentials", + "cumulusci.core.tasks.BaseSalesforceTask._update_credentials", MagicMock(return_value=None), ) class TestRunBatchApex(MockLoggerMixin): diff --git a/cumulusci/tasks/salesforce/tests/test_base_tasks.py b/cumulusci/tasks/salesforce/tests/test_base_tasks.py index 9769066342..d30461b3f5 100644 --- a/cumulusci/tasks/salesforce/tests/test_base_tasks.py +++ b/cumulusci/tasks/salesforce/tests/test_base_tasks.py @@ -28,9 +28,7 @@ def setup_method(self): self.org_config = OrgConfig({}, "test", keychain=self.project_config.keychain) def test_run_task(self): - with mock.patch( - "cumulusci.tasks.salesforce.BaseSalesforceTask._update_credentials" - ): + with mock.patch("cumulusci.core.tasks.BaseSalesforceTask._update_credentials"): task = BaseSalesforceTask( self.project_config, self.task_config, self.org_config ) diff --git a/cumulusci/tasks/salesforce/tests/test_network_member_group.py b/cumulusci/tasks/salesforce/tests/test_network_member_group.py index 554a046d03..cf210b6c6c 100644 --- a/cumulusci/tasks/salesforce/tests/test_network_member_group.py +++ b/cumulusci/tasks/salesforce/tests/test_network_member_group.py @@ -247,8 +247,7 @@ def test_create_network_member_group__parent_not_found_in_query(self): task.sf = Mock() - insert_response = Mock() - task.sf.NetworkMemberGroup.create = Mock(insert_response) + task.sf.NetworkMemberGroup.create = Mock() # Execute the test. with pytest.raises(CumulusCIException) as context: @@ -278,9 +277,7 @@ def test_create_network_member_group__parent_already_exists(self): task.sf = Mock() task.sf.NetworkMemberGroup = Mock() - insert_response = Mock() - insert_response.get = Mock() - task.sf.NetworkMemberGroup.create = Mock(insert_response) + task.sf.NetworkMemberGroup.create = Mock() # Execute the test. task._create_network_member_group(sobject_type, parent_name, parent_id) diff --git a/cumulusci/tasks/salesforce/tests/util.py b/cumulusci/tasks/salesforce/tests/util.py index 02b529e2a4..9acc330e8a 100644 --- a/cumulusci/tasks/salesforce/tests/util.py +++ b/cumulusci/tasks/salesforce/tests/util.py @@ -27,7 +27,7 @@ def create_task(task_class, options=None, project_config=None, org_config=None): options = {} task_config = TaskConfig({"options": options}) with mock.patch( - "cumulusci.tasks.salesforce.BaseSalesforceTask._get_client_name", + "cumulusci.core.tasks.BaseSalesforceTask._get_client_name", return_value="ccitests", ): return task_class(project_config, task_config, org_config) diff --git a/cumulusci/tasks/tests/test_salesforce.py b/cumulusci/tasks/tests/test_salesforce.py index 55e65e0dca..9627f9e737 100644 --- a/cumulusci/tasks/tests/test_salesforce.py +++ b/cumulusci/tasks/tests/test_salesforce.py @@ -12,7 +12,7 @@ @patch( - "cumulusci.tasks.salesforce.BaseSalesforceTask._update_credentials", + "cumulusci.core.tasks.BaseSalesforceTask._update_credentials", MagicMock(return_value=None), ) class TestSalesforceToolingTask: diff --git a/cumulusci/tests/util.py b/cumulusci/tests/util.py index d221ede8c4..a16294be74 100644 --- a/cumulusci/tests/util.py +++ b/cumulusci/tests/util.py @@ -253,7 +253,7 @@ def _init_task(): task.sf = salesforce_client with mock.patch( - "cumulusci.core.config.OrgConfig.is_person_accounts_enabled", + "cumulusci.core.config.org_config.OrgConfig.is_person_accounts_enabled", lambda: is_person_accounts_enabled, ), mock.patch.object(task, "_init_task", _init_task): yield From 37f34dccb2855ce5afe22c8b579181e68b2cd64c Mon Sep 17 00:00:00 2001 From: James Estevez Date: Wed, 30 Nov 2022 18:19:44 -0800 Subject: [PATCH 3/4] Update create_task_fixture for 3.11 Python 3.11 removed pathlib._Accessor in python/cpython 25701. This broke our patch when it replaced a call to `os.getcwd()` with `Path.cwd()` in `Path.absolute()`. As a classmethod `Path.cwd()` receives the class as its implicit first argument meaning we can have 0 or 1 arguments. --- cumulusci/tasks/salesforce/tests/util.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/cumulusci/tasks/salesforce/tests/util.py b/cumulusci/tasks/salesforce/tests/util.py index 9acc330e8a..7557adf91d 100644 --- a/cumulusci/tasks/salesforce/tests/util.py +++ b/cumulusci/tasks/salesforce/tests/util.py @@ -2,7 +2,8 @@ from tempfile import TemporaryDirectory from unittest import mock -from cumulusci.core.config import OrgConfig, TaskConfig +from cumulusci.core.config import TaskConfig +from cumulusci.core.config.org_config import OrgConfig from cumulusci.tests.util import DummyKeychain, create_project_config @@ -34,9 +35,17 @@ def create_task(task_class, options=None, project_config=None, org_config=None): def patch_dir(patch_path, file_path): + def return_file_path(*args, **kwargs): + """ + Python 3.11 removed pathlib._Accessor. This replaced a call to + os.getcwd() with Path.cwd() in Path.absolute(). This means we can have 0 + or 1 arguments. + """ + return file_path + directory = Path(file_path) directory.mkdir(parents=True, exist_ok=True) - patch = mock.patch(patch_path, lambda: file_path) + patch = mock.patch(patch_path, return_file_path) patch.start() return patch From 8bbf6ca02ef9e8744aa8d12f7937788f5476a520 Mon Sep 17 00:00:00 2001 From: James Estevez Date: Wed, 30 Nov 2022 16:15:08 -0800 Subject: [PATCH 4/4] Update enum assert for 3.11 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The behavior of string enums has changed in 3.11 [what's new]: > Changed Enum.__format__() (the default for format(), str.format() and > f-strings) of enums with mixed-in types (e.g. int, str) to also include > the class name in the output, not just the member’s key. This matches > the existing behavior of enum.Enum.__str__(), returning e.g. > 'AnEnum.MEMBER' for an enum AnEnum(str, Enum) instead of just 'MEMBER'. This almost certainly explains the packaging bugs we've seen. [what's new]: https://docs.python.org/3.11/whatsnew/3.11.html#enum --- cumulusci/tasks/bulkdata/tests/test_upsert.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cumulusci/tasks/bulkdata/tests/test_upsert.py b/cumulusci/tasks/bulkdata/tests/test_upsert.py index 63acde98a3..9ac5111ef6 100644 --- a/cumulusci/tasks/bulkdata/tests/test_upsert.py +++ b/cumulusci/tasks/bulkdata/tests/test_upsert.py @@ -232,7 +232,9 @@ def test_upsert_rest__faked( relevant_debug_statement = look_for_operation_creation_debug_statement( task.logger.debug.mock_calls ) - assert relevant_debug_statement == DataApi.REST, relevant_debug_statement + assert relevant_debug_statement == format( + DataApi.REST + ), relevant_debug_statement def _mock_bulk(self, domain): responses.add( @@ -419,7 +421,9 @@ def test_upsert__fake_bulk(self, create_task, cumulusci_test_repo_root, org_conf relevant_debug_statement = look_for_operation_creation_debug_statement( task.logger.debug.mock_calls ) - assert relevant_debug_statement == DataApi.BULK, relevant_debug_statement + assert relevant_debug_statement in format( + DataApi.BULK + ), relevant_debug_statement def _test_two_upserts_and_check_results__complex( self, api, create_task, cumulusci_test_repo_root, sf