-
Notifications
You must be signed in to change notification settings - Fork 28
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
Rabbit sync new #120
base: rabbit-sync
Are you sure you want to change the base?
Rabbit sync new #120
Conversation
added _wrap_middleware empty implementation
def test_init_connect_by_url(self, settings): | ||
args, kwargs = self.get_broker_args(settings) | ||
broker = self.broker(*args, **kwargs) | ||
assert broker.connect() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please refactor all the following testcases to use a mocked _connect
method instead? We can check incoming *args, **kwargs
arguments by assert_called_with
method, so we need no more connect to the real broker to test correct arguments merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this ok? Or do you have something else in mind?
@patch("pika.BlockingConnection")
def test_init_connect(self, mock, settings: Settings):
mock_conn = MagicMock()
mock.return_value = mock_conn
args, kwargs = self.get_broker_args(settings)
broker = self.broker(*args, **kwargs)
broker.connect()
mock.assert_called_once_with(
pika.URLParameters(settings.url),
)
broker.close()
mock_conn.close.assert_called_once()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about replacing the original broker _connect
method to check incoming arguments. It will be suitable for all brokers, not RMQ only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give me an example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tomorrow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By words: you need to patch this one https://github.com/Lancetnik/Propan/blob/main/propan/brokers/rabbit/rabbit_broker.py#L108 the way you patching in the TestBroker
'broker._connect = MethodType(async_mock, broker)' and check the mock call arguments
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
Checklist
scripts/lint.sh
has no errors)