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

Refactor: Extract RabbitMQ management client & provide fakes #89

Merged
merged 2 commits into from
Mar 30, 2021

Conversation

coro
Copy link
Contributor

@coro coro commented Mar 30, 2021

This came from my looking into #88. Our only tests that touched the controller also used a real RabbitMQ, so it was not possible to control the responses from the RabbitMQ API in these tests, and force the API to respond with a 401.

This extracts all of the rabbithole client logic into a factory, which creates an object providing all of the rabbithole methods that we use in our controllers. This also adds counterfeiter methods to create fakes for all of these methods, allowing us to write more comprehensive integration tests at the per-controller level.

In the actual binary, we use a factory to generate a real rabbithole client. In the tests, we can now create our own mock factory which creates a fake RabbitMQ management client, which can return whatever we choose in the tests.

I will use this framework to address #88 by writing a test to return an error response from rabbithole, and triggering the panic hit in that error.

Additional considerations

This means we will have to add to the interface and regenerate the fake code whenever we use a new rabbithole method. We can always add a make target for the autogen side of this if needs be.

coro added 2 commits March 30, 2021 12:33
Allows for fakes to be used in integration tests to mock out the
rabbitmq server being referenced by the controller
Also implement the ClientFactory in the controllers
@coro coro merged commit aed2478 into main Mar 30, 2021
@coro coro deleted the refactor-rabbithole branch March 30, 2021 16:38
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