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

Adjustments to interfaces to improve the mockability and faking of NATS for unit testing in consumer apps. #654

Merged
merged 6 commits into from
Aug 8, 2022

Conversation

sspates
Copy link
Contributor

@sspates sspates commented Aug 5, 2022

Updated a few interfaces to use IConnection instead of Connection as Connection is internal. This is preventing mocking libraries from being able to mock properly.
Added interface for ConnectionFactory so factory can be mocked/faked in unit tests.

Copy link
Collaborator

@scottf scottf left a comment

Choose a reason for hiding this comment

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

I like this idea but we cannot change the interface in a non backward compatible way.

src/NATS.Client/ISubscription.cs Outdated Show resolved Hide resolved
src/NATS.Client/Subscription.cs Outdated Show resolved Hide resolved
@sspates
Copy link
Contributor Author

sspates commented Aug 5, 2022

@scottf I removed the IConnection changes, but found that protecting the Message in the handler makes mocking the handling of the message impossible. I adjusted the Handler to expose the message to being set.

@scottf scottf requested review from caleblloyd and removed request for caleblloyd August 6, 2022 20:03
src/NATS.Client/Msg.cs Outdated Show resolved Hide resolved
@scottf
Copy link
Collaborator

scottf commented Aug 6, 2022

@sspates-starbucks I'm good with the event message changes, not excited about having a public setter, but I suppose it's benign, the worst that could happen is the dev overwrites their own message.
Frankly I'm not seeing the need for the IConnectionFactory interface. There is already an IConnection interface so you can provide your own mock connection if you want to.
And I'll mention again the ability to run a server locally during tests. In .net integration tests we generally start a server for each suite, but for java we start a server for each test.

Copy link
Collaborator

@scottf scottf left a comment

Choose a reason for hiding this comment

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

LGTM

@scottf scottf merged commit 74e8e92 into nats-io:master Aug 8, 2022
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.

3 participants