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

Fix asserts for called once in Python 3.12 #2448

Merged
merged 2 commits into from
Oct 12, 2023

Conversation

hrnciar
Copy link
Contributor

@hrnciar hrnciar commented Jul 27, 2023

Since Python 3.12 the tests fail with: AttributeError: 'called_once_with' is not a valid assertion. Use a spec for the mock if 'called_once_with' is meant to be an attribute.. Did you mean: 'assert_called_once_with'?

While attempting to fix them we find out that the tests were not asserting anything in the past. This PR tries to solve it.

@carlosperate carlosperate changed the base branch from master to pyqt6 October 12, 2023 13:57
@carlosperate carlosperate changed the base branch from pyqt6 to master October 12, 2023 13:57
@carlosperate
Copy link
Member

carlosperate commented Oct 12, 2023

Thanks for the PR @hrnciar, not sure why the tests didn't run, even if I changed the base branch (sometime that helps GitHub reset PRs). So I've rebased and pushed to your branch, and now it looks like the CI has been finally triggered.

Once the tests pass we can merge the PR 👍

@@ -2524,7 +2524,7 @@ def test_change_mode_workspace_dir_exception():
ed.change_mode("circuitpython")
assert mock_error.call_count == 1
assert ed.mode == "circuitpython"
assert python_mode.workspace_dir.called_once()
python_mode.workspace_dir.assert_called_once()
Copy link
Member

Choose a reason for hiding this comment

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

Looks like assert_called_once() was introduced in Python 3.6, so it fails in 3.5.
We are about to update dependencies and drop support for all the legacy stuff, but in the meantime I'll update this to use assert_called_once_with() and it should pass tests.

tests/test_logic.py Outdated Show resolved Hide resolved
Copy link
Member

@carlosperate carlosperate left a comment

Choose a reason for hiding this comment

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

Okay, so the only CI failure is a known issue with the PiOS stretch environment, so this is good to be merged 👍

Thanks @hrnciar!

@carlosperate carlosperate merged commit 3776e0c into mu-editor:master Oct 12, 2023
30 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants