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 mock callable/constructor to not accept extra configuration after… #241

Merged
merged 2 commits into from
Sep 12, 2020

Conversation

ldfsilva
Copy link
Contributor

What:

This fixes issue 16, not allowing that extra configuration is made after mock_callable, mock_async_callable or mock_constructor` receive their first call.

Added tests to cover the constraint to not allow extra configuration to be made in mock_callable, mock_async_callable and mock_constructor and also validated with ad-hoc tests that TestSlide tests continues to behave as expected. As show below:

For the ad-hoc tests I used this test_extra_configuration.py and invoked tests as follows:

python -m unittest -v tests.test_extra_configuration

Here are excerpts from the test results: [here]

mock_async_calable:

ERROR: test_mock_async_callable_fail_already_received_1_call (tests.test_extra_configuration.TestCase)
ValueError: No extra configuration is allowed after mock_async_callable receives its first call, it received 1 call already. You should instead define it all at once, eg: self.mock_async_callable(target, 'func').to_return_value(value).and_assert_called_once()
ERROR: test_mock_async_callable_fail_already_received_8_calls (tests.test_extra_configuration.TestCase)
ValueError: No extra configuration is allowed after mock_async_callable receives its first call, it received 8 calls already. You should instead define it all at once, eg: self.mock_async_callable(target, 'func').to_return_value(value).and_assert_called_once()

mock_calable:

ERROR: test_mock_callable_fail_already_received_1_call (tests.test_extra_configuration.TestCase)
ValueError: No extra configuration is allowed after mock_callable receives its first call, it received 1 call already. You should instead define it all at once, eg: self.mock_callable(target, 'func').to_return_value(value).and_assert_called_once()
ERROR: test_mock_callable_fail_already_received_8_calls (tests.test_extra_configuration.TestCase)
ValueError: No extra configuration is allowed after mock_callable receives its first call, it received 8 calls already. You should instead define it all at once, eg: self.mock_callable(target, 'func').to_return_value(value).and_assert_called_once()

mock_constructor:

ERROR: test_mock_constructor_fail_already_received_1_call (tests.test_extra_configuration.TestCase)
ValueError: No extra configuration is allowed after mock_constructor receives its first call, it received 1 call already. You should instead define it all at once, eg: self.mock_constructor(target, 'func').to_return_value(value).and_assert_called_once()
ERROR: test_mock_constructor_fail_already_received_8_calls (tests.test_extra_configuration.TestCase)
ValueError: No extra configuration is allowed after mock_constructor receives its first call, it received 8 calls already. You should instead define it all at once, eg: self.mock_constructor(target, 'func').to_return_value(value).and_assert_called_once()

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 12, 2020
@deathowl
Copy link
Member

LGTM. @ldfsilva Just fix the breaking testcase plase :)

@ldfsilva
Copy link
Contributor Author

sounds good, I just noticed that we have a designated test file for mock_async_callable (need glasses), so I relocated that test over there, removed asyncio.run and fixed linter, it should be good now.

@deathowl deathowl merged commit 8b8b23a into facebook:master Sep 12, 2020

def _validate_patch(
self,
name="mock_callable",
name=_NAME,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can drop the name argument here, as it can (should!) be accessed with self._NAME.

@fornellas
Copy link
Contributor

Thanks @ldfsilva for working on this! Small, focused and with awesome simple test coverage!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fail mock_callable / mock_constructor configuration if it already received a call
4 participants