Cleanup some warnings (#3491)

Cleanup a few warnings. I'll use comments to explain.

---------

Co-authored-by: Paul Prescod <pprescod@salesforce.com>
Co-authored-by: James Estevez <jestevez@salesforce.com>

Author: 3 people

.prettierignore

@@ -0,0 +1 @@
+Test*.yaml

cumulusci/core/config/org_config.py

@@ -3,6 +3,7 @@
 from collections import defaultdict, namedtuple
 from contextlib import contextmanager
 from datetime import date, datetime
+from typing import Optional
 from urllib.parse import urlparse
 
 import requests
@@ -47,14 +48,12 @@ class OrgConfig(BaseConfig):
     is_sandbox: bool
     namespace: str
     namespaced: bool
-    org_id: str
     org_type: str
     password: str
     scratch: bool
     scratch_org_type: str
     set_password: bool
     sfdx_alias: str
-    username: str
     userinfo: str
     id: str
     active: bool
@@ -63,8 +62,9 @@ class OrgConfig(BaseConfig):
     refresh_token: str
     client_secret: str
     connected_app: str
+    serialization_format: str
 
-    createable: bool = None
+    createable: Optional[bool] = None
 
     # make sure it can be mocked for tests
     OAuth2Client = OAuth2Client
@@ -204,7 +204,15 @@ def user_id(self):
 
     @property
     def org_id(self):
-        return self.id.split("/")[-2]
+        try:
+            if org_id := self.config.get("org_id"):
+                return org_id
+            elif hasattr(self, "id") and self.id:
+                return self.id.split("/")[-2]
+            else:
+                return None
+        except Exception as e:  # pragma: no cover
+            assert e is None, e
 
     @property
     def username(self):
@@ -254,7 +262,7 @@ def populate_expiration_date(self):
     @property
     def organization_sobject(self):
         """Cached copy of Organization sObject. Does not perform API call."""
-        return self._org_sobject
+        return getattr(self, "_org_sobject", None)
 
     def _fetch_community_info(self):
         """Use the API to re-fetch information about communities"""
@@ -317,7 +325,8 @@ def installed_packages(self):
         To check if a required package is present, call `has_minimum_package_version()` with either the
         namespace or 033 Id of the desired package and its version, in 1.2.3 format.
 
-        Beta version of a package are represented as "1.2.3b5", where 5 is the build number."""
+        Beta version of a package are represented as "1.2.3b5", where 5 is the build number.
+        """
         if self._installed_packages is None:
             isp_result = self.salesforce_client.restful(
                 "tooling/query/?q=SELECT SubscriberPackage.Id, SubscriberPackage.NamespacePrefix, "

cumulusci/core/config/tests/test_config.py

@@ -66,7 +66,7 @@ def test_getattr_toplevel_key_missing(self):
             assert config.foo is None
         with mock.patch(
             "cumulusci.core.config.base_config.STRICT_GETATTR", True
-        ), pytest.raises(AssertionError):
+        ), pytest.deprecated_call(), pytest.raises(AssertionError):
             assert config.foo is None
 
     def test_getattr_child_key(self):

cumulusci/core/dependencies/tests/test_dependencies.py

@@ -645,6 +645,7 @@ def test_install(self, api_deploy_mock, zip_builder_mock, download_mock):
         assert mock_task.project_config == context
 
         api_deploy_mock.return_value.assert_called_once()
+        zf.close()
 
     def test_get_unmanaged(self):
         org = mock.Mock()
@@ -733,6 +734,7 @@ def test_install(self, api_deploy_mock, zip_builder_mock, download_mock):
         assert mock_task.project_config == context
 
         api_deploy_mock.return_value.assert_called_once()
+        zf.close()
 
     def test_get_unmanaged(self):
         org = mock.Mock()
@@ -793,6 +795,7 @@ def test_get_metadata_package_zip_builder__mdapi_root(
             },
             context=mock.ANY,
         )
+        zf.close()
 
     @mock.patch("cumulusci.core.dependencies.dependencies.MetadataPackageZipBuilder")
     @mock.patch("cumulusci.core.dependencies.dependencies.download_extract_zip")
@@ -827,6 +830,7 @@ def test_get_metadata_package_zip_builder__mdapi_subfolder(
             },
             context=mock.ANY,
         )
+        zf.close()
 
     @mock.patch("cumulusci.core.dependencies.dependencies.MetadataPackageZipBuilder")
     @mock.patch("cumulusci.core.dependencies.dependencies.download_extract_zip")
@@ -866,6 +870,7 @@ def test_get_metadata_package_zip_builder__sfdx(
             capture_output=True,
             check_return=True,
         )
+        zf.close()
 
 
 class TestParseDependency:

cumulusci/oauth/tests/test_client.py

@@ -13,7 +13,10 @@
 import responses
 from requests.models import Response
 
-from cumulusci.core.exceptions import SalesforceCredentialsException
+from cumulusci.core.exceptions import (
+    CumulusCIUsageError,
+    SalesforceCredentialsException,
+)
 from cumulusci.core.keychain.base_project_keychain import DEFAULT_CONNECTED_APP_PORT
 from cumulusci.oauth.client import (
     PORT_IN_USE_ERR,
@@ -72,9 +75,17 @@ def http_client(client_config):
 
 @contextmanager
 @mock.patch("time.sleep", time.sleep)  # undo mock from conftest
-def httpd_thread(oauth_client):
+def httpd_thread(oauth_client, expected_error=None):
     # call OAuth object on another thread - this spawns local httpd
-    thread = threading.Thread(target=oauth_client.auth_code_flow)
+
+    def run_code_and_check_exception():
+        if expected_error:
+            with pytest.raises(expected_error):
+                oauth_client.auth_code_flow()
+        else:
+            oauth_client.auth_code_flow()
+
+    thread = threading.Thread(target=run_code_and_check_exception)
     thread.start()
     while thread.is_alive():
         if oauth_client.httpd:
@@ -192,7 +203,7 @@ def test_oauth_flow_error_from_auth(self, client):
         )
 
         # call OAuth object on another thread - this spawns local httpd
-        with httpd_thread(client):
+        with httpd_thread(client, OAuth2Error):
             # simulate callback from browser
             with pytest.raises(urllib.error.HTTPError):
                 urllib.request.urlopen(
@@ -204,7 +215,7 @@ def test_oauth_flow_error_from_auth(self, client):
         sys.platform.startswith("win"), reason="setup differs from windows"
     )
     def test_create_httpd__port_already_in_use(self, client):
-        with httpd_thread(client):
+        with httpd_thread(client, CumulusCIUsageError):
             with pytest.raises(
                 OAuth2Error, match=PORT_IN_USE_ERR.format(DEFAULT_CONNECTED_APP_PORT)
             ):
@@ -227,7 +238,7 @@ def test_oauth_flow_error_from_token(self, client):
         )
 
         # call OAuth object on another thread - this spawns local httpd
-        with httpd_thread(client):
+        with httpd_thread(client, OAuth2Error):
             # simulate callback from browser
             with pytest.raises(urllib.error.HTTPError):
                 urllib.request.urlopen(client.client_config.redirect_uri + "?code=123")

cumulusci/tasks/apex/tests/test_apex_tasks.py

@@ -8,6 +8,7 @@
 
 import pytest
 import responses
+from responses.matchers import query_string_matcher
 from simple_salesforce import SalesforceGeneralError
 
 from cumulusci.core import exceptions as exc
@@ -73,9 +74,9 @@ def setup_method(self):
 
     def _mock_apex_class_query(self, name="TestClass_TEST", namespace=None):
         namespace_param = "null" if namespace is None else f"%27{namespace}%27"
-        url = (
-            self.base_tooling_url
-            + "query/?q=SELECT+Id%2C+Name+"
+        url = self.base_tooling_url + "query/"
+        query_string = (
+            "q=SELECT+Id%2C+Name+"
             + f"FROM+ApexClass+WHERE+NamespacePrefix+%3D+{namespace_param}"
             + "+AND+%28Name+LIKE+%27%25_TEST%27%29"
         )
@@ -85,7 +86,10 @@ def _mock_apex_class_query(self, name="TestClass_TEST", namespace=None):
             "totalSize": 1,
         }
         responses.add(
-            responses.GET, url, match_querystring=True, json=expected_response
+            responses.GET,
+            url,
+            match=[query_string_matcher(query_string)],
+            json=expected_response,
         )
 
     def _get_mock_test_query_results(self, methodnames, outcomes, messages):
@@ -163,16 +167,14 @@ def _get_mock_test_query_results(self, methodnames, outcomes, messages):
 
     def _get_mock_test_query_url(self, job_id):
         return (
-            self.base_tooling_url
-            + "query/?q=%0ASELECT+Id%2CApexClassId%2CTestTimestamp%2C%0A+++++++Message%2CMethodName%2COutcome%2C%0A+++++++RunTime%2CStackTrace%2C%0A+++++++%28SELECT%0A++++++++++Id%2CCallouts%2CAsyncCalls%2CDmlRows%2CEmail%2C%0A++++++++++LimitContext%2CLimitExceptions%2CMobilePush%2C%0A++++++++++QueryRows%2CSosl%2CCpu%2CDml%2CSoql%0A++++++++FROM+ApexTestResults%29%0AFROM+ApexTestResult%0AWHERE+AsyncApexJobId%3D%27{}%27%0A".format(
-                job_id
-            )
+            self.base_tooling_url + "query/",
+            f"q=%0ASELECT+Id%2CApexClassId%2CTestTimestamp%2C%0A+++++++Message%2CMethodName%2COutcome%2C%0A+++++++RunTime%2CStackTrace%2C%0A+++++++%28SELECT%0A++++++++++Id%2CCallouts%2CAsyncCalls%2CDmlRows%2CEmail%2C%0A++++++++++LimitContext%2CLimitExceptions%2CMobilePush%2C%0A++++++++++QueryRows%2CSosl%2CCpu%2CDml%2CSoql%0A++++++++FROM+ApexTestResults%29%0AFROM+ApexTestResult%0AWHERE+AsyncApexJobId%3D%27{job_id}%27%0A",
         )
 
     def _get_mock_testqueueitem_status_query_url(self, job_id):
         return (
-            self.base_tooling_url
-            + f"query/?q=SELECT+Id%2C+Status%2C+ExtendedStatus%2C+ApexClassId+FROM+ApexTestQueueItem+WHERE+ParentJobId+%3D+%27{job_id}%27+AND+Status+%3D+%27Failed%27"
+            (self.base_tooling_url + "query/"),
+            f"q=SELECT+Id%2C+Status%2C+ExtendedStatus%2C+ApexClassId+FROM+ApexTestQueueItem+WHERE+ParentJobId+%3D+%27{job_id}%27+AND+Status+%3D+%27Failed%27",
         )
 
     def _mock_get_test_results(
@@ -182,44 +184,50 @@ def _mock_get_test_results(
         job_id="JOB_ID1234567",
         methodname=["TestMethod"],
     ):
-        url = self._get_mock_test_query_url(job_id)
+        url, query_string = self._get_mock_test_query_url(job_id)
 
         expected_response = self._get_mock_test_query_results(
             methodname, [outcome], [message]
         )
         responses.add(
-            responses.GET, url, match_querystring=True, json=expected_response
+            responses.GET,
+            url,
+            match=[query_string_matcher(query_string)],
+            json=expected_response,
         )
 
     def _mock_get_test_results_multiple(
         self, method_names, outcomes, messages, job_id="JOB_ID1234567"
     ):
-        url = self._get_mock_test_query_url(job_id)
+        url, query_string = self._get_mock_test_query_url(job_id)
 
         expected_response = self._get_mock_test_query_results(
             method_names, outcomes, messages
         )
         responses.add(
-            responses.GET, url, match_querystring=True, json=expected_response
+            responses.GET,
+            url,
+            match=[query_string_matcher(query_string)],
+            json=expected_response,
         )
 
     def _mock_get_failed_test_classes(self, job_id="JOB_ID1234567"):
-        url = self._get_mock_testqueueitem_status_query_url(job_id)
+        url, query_string = self._get_mock_testqueueitem_status_query_url(job_id)
 
         responses.add(
             responses.GET,
             url,
-            match_querystring=True,
+            match=[query_string_matcher(query_string)],
             json={"totalSize": 0, "records": [], "done": True},
         )
 
     def _mock_get_failed_test_classes_failure(self, job_id="JOB_ID1234567"):
-        url = self._get_mock_testqueueitem_status_query_url(job_id)
+        url, query_string = self._get_mock_testqueueitem_status_query_url(job_id)
 
         responses.add(
             responses.GET,
             url,
-            match_querystring=True,
+            match=[query_string_matcher(query_string)],
             json={
                 "totalSize": 1,
                 "records": [
@@ -235,14 +243,15 @@ def _mock_get_failed_test_classes_failure(self, job_id="JOB_ID1234567"):
         )
 
     def _mock_get_symboltable(self):
-        url = (
-            self.base_tooling_url
-            + "query/?q=SELECT+SymbolTable+FROM+ApexClass+WHERE+Name%3D%27TestClass_TEST%27"
+        url = self.base_tooling_url + "query/"
+        query_string = (
+            "q=SELECT+SymbolTable+FROM+ApexClass+WHERE+Name%3D%27TestClass_TEST%27"
         )
 
         responses.add(
             responses.GET,
             url,
+            match=[query_string_matcher(query_string)],
             json={
                 "records": [
                     {
@@ -265,9 +274,9 @@ def _mock_get_symboltable_failure(self):
         responses.add(responses.GET, url, json={"records": []})
 
     def _mock_tests_complete(self, job_id="JOB_ID1234567"):
-        url = (
-            self.base_tooling_url
-            + "query/?q=SELECT+Id%2C+Status%2C+"
+        url = self.base_tooling_url + "query/"
+        query_string = (
+            "q=SELECT+Id%2C+Status%2C+"
             + "ApexClassId+FROM+ApexTestQueueItem+WHERE+ParentJobId+%3D+%27"
             + "{}%27".format(job_id)
         )
@@ -277,23 +286,29 @@ def _mock_tests_complete(self, job_id="JOB_ID1234567"):
             "records": [{"Status": "Completed"}],
         }
         responses.add(
-            responses.GET, url, match_querystring=True, json=expected_response
+            responses.GET,
+            url,
+            match=[query_string_matcher(query_string)],
+            json=expected_response,
         )
 
     def _mock_tests_processing(self, job_id="JOB_ID1234567"):
-        url = (
-            self.base_tooling_url
-            + "query/?q=SELECT+Id%2C+Status%2C+"
+        url = self.base_tooling_url + "query/"
+        query_string = (
+            "q=SELECT+Id%2C+Status%2C+"
             + "ApexClassId+FROM+ApexTestQueueItem+WHERE+ParentJobId+%3D+%27"
-            + "{}%27".format(job_id)
+            + f"{job_id}%27"
         )
         expected_response = {
             "done": True,
             "totalSize": 1,
             "records": [{"Status": "Processing", "ApexClassId": 1}],
         }
         responses.add(
-            responses.GET, url, match_querystring=True, json=expected_response
+            responses.GET,
+            url,
+            match=[query_string_matcher(query_string)],
+            json=expected_response,
         )
 
     def _mock_run_tests(self, success=True, body="JOB_ID1234567"):

cumulusci/tasks/bulkdata/tests/test_snowfakery.py

@@ -783,12 +783,11 @@ def test_explicit_channel_declarations(self, mock_load_data, create_task):
                 "recipe": Path(__file__).parent
                 / "snowfakery/simple_snowfakery.recipe.yml",
                 "run_until_recipe_repeated": 15,
-                "recipe_options": {"xyzzy": "Nothing happens", "some_number": 42},
                 "loading_rules": Path(__file__).parent
                 / "snowfakery/simple_snowfakery_channels.load.yml",
             },
         )
-        with mock.patch.object(
+        with pytest.warns(UserWarning), mock.patch.object(
             task.project_config, "keychain", DummyKeychain()
         ) as keychain:
 
@@ -833,7 +832,6 @@ def test_serial_mode(self, mock_load_data, create_task):
                 "recipe": Path(__file__).parent
                 / "snowfakery/simple_snowfakery.recipe.yml",
                 "run_until_recipe_repeated": 15,
-                "recipe_options": {"xyzzy": "Nothing happens", "some_number": 42},
                 "bulk_mode": "Serial",
             },
         )