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

Cleanup some warnings #3491

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Cleanup some warnings #3491

wants to merge 9 commits into from

Conversation

prescod
Copy link
Contributor

@prescod prescod commented Jan 16, 2023

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

@prescod prescod requested a review from a team as a code owner January 16, 2023 22:08
@prescod prescod added the ignore-for-release Don't include in release notes label Jan 17, 2023
.prettierignore Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't bother prettying Cassettes

@@ -204,7 +205,15 @@ def user_id(self):

@property
def org_id(self):
return self.id.split("/")[-2]
try:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code probably relied previously on implicit None for config objects which is evil.

@@ -254,7 +263,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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code probably relied previously on implicit None for config objects which is evil.

@@ -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):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capture a deprecated call.

@@ -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()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Close a file explicitly.

responses.GET, url, match_querystring=True, json=expected_response
responses.GET,
url,
match=[query_string_matcher(query_string)],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Deprecate match_querystring argument in Response and CallbackResponse.
    Use responses.matchers.query_param_matcher or responses.matchers.query_string_matcher

)

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/"),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switch to query_string_matcher

@@ -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},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was cut and paste from elsewhere and didn't need to be tested over and over.

@@ -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},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was cut and paste from elsewhere and didn't need to be tested over and over.

@@ -186,7 +186,7 @@ class GithubIssuesParser(IssuesParser):

def __new__(cls, release_notes_generator, title, issue_regex=None):
if not release_notes_generator.has_issues:
logging.getLogger(__file__).warn(
logging.getLogger(__file__).warning(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As of Python 3.3, logger.warn has been deprecated and logger.warning must be used. Even though logger.warn is still backwards compatible, you should not use it.

Copy link

Thanks for the contribution! Unfortunately we can't verify the commit author(s): Paul Prescod <p***@s***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, refresh the status of this Pull Request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:missing ignore-for-release Don't include in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants