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

Facilitate unit testing of client code of nats.net.v2 #105

Closed
jasper-d opened this issue Jul 31, 2023 · 7 comments
Closed

Facilitate unit testing of client code of nats.net.v2 #105

jasper-d opened this issue Jul 31, 2023 · 7 comments
Labels
enhancement New feature or request testing

Comments

@jasper-d
Copy link
Contributor

jasper-d commented Jul 31, 2023

Feature Request

Make testing easier by using interfaces instead of sealed classes.

Use Case:

Writing tests for code that uses nats.net.v2 is challenging, because the types nats.net.v2 exposes are sealed classes which cannot be replaced with test doubles.

Proposed Change:

Define interfaces for at least NatsConnectionPool and NatsConnection to facilitate the use of test doubles so that users of nats.net.v2 can easily replace those and test their code without requiring nats-server.

+ public interface INatsConnectionPool : IAsyncDisposable {
+  INatsConnection GetConnection();
+  IEnumerable<INatsConnection> GetConnections();
+  INatsConnection GetCommand(); // I'm wondering why this is named GetCommand but that's a separate issue
+}

- public sealed class NatsConnectionPool : IAsyncDisposable
+ public sealed class NatsConnectionPool : INatsConnectionPool

// Similar changes for NatsConnection

Who Benefits From The Change(s)?

Every user of nats.net.v2 who wants to test their code.

Alternative Approaches

Have nats-server configured and running when executing tests.

@mtmk
Copy link
Collaborator

mtmk commented Aug 1, 2023

I don't see why we shouldn't create an INatsConnectionPool interface.

We already have an INatsConnection interface, but extension methods (I think just request-reply methods) works with NatsConnection due to internal calls. We'd need to revisit that.

As for running nats-server for tests, we already have a test utility for that. Do you think it would be handy to publish the test utility on NuGet?

@mtmk mtmk added the testing label Aug 1, 2023
@jasper-d
Copy link
Contributor Author

jasper-d commented Aug 1, 2023

I don't see why we shouldn't create an INatsConnectionPool interface.

Great, I can spend some time on it if you don't mind and hopefully come up with a proper proposal.

We already have an INatsConnection interface, but extension methods (I think just request-reply methods) works with NatsConnection due to internal calls. We'd need to revisit that.

I found NatsRequestExtensions and NatsRequestManyExtensions. Technically those methods could be moved to INatsConnection and become ordinary instance methods I suppose. But there is probably a reason for them being extension methods.

Alternatively, the few internal calls used by the extension methods could possibly be made public and become part of the interface. Mocking would then work, although it can be a bit cumbersome to setup.

As for running nats-server for tests, we already have a test utility for that. Do you think it would be handy to publish the test utility on NuGet?

From my point of view that isn't necessary and would potentially make it more complicated to evolve the implementation/its interface for this repositories purposes. For real e2e tests I usually setup a container running nats-server (instead of requiring nats-server included in PATH) - but others may have different needs.

My motivation is to easily stub out all IO to quickly and reliably test business logic w/o external dependencies, simulate IO failures/timeouts etc.

@mtmk
Copy link
Collaborator

mtmk commented Aug 1, 2023

I don't see why we shouldn't create an INatsConnectionPool interface.

Great, I can spend some time on it if you don't mind and hopefully come up with a proper proposal.

That'd be brilliant! Could be a quick PR if you're happy with that.

We already have an INatsConnection interface, but extension methods (I think just request-reply methods) works with NatsConnection due to internal calls. We'd need to revisit that.

I found NatsRequestExtensions and NatsRequestManyExtensions. Technically those methods could be moved to INatsConnection and become ordinary instance methods I suppose. But there is probably a reason for them being extension methods.

Alternatively, the few internal calls used by the extension methods could possibly be made public and become part of the interface. Mocking would then work, although it can be a bit cumbersome to setup.

The idea is to keep public interfaces as succinct as possible. @caleblloyd's input would be invaluable here.

@scottf
Copy link
Contributor

scottf commented Aug 2, 2023

I'm not disagreeing about having interfaces, but our clients run unit tests with actual servers. The java one starts a new server for every single test, the .NET V1 does something similar. It's imperceptibly slower. If you don't like running a server for every test, you can run an instance per suite of tests. I would say why not have a real server running since you can.

@caleblloyd
Copy link
Collaborator

We already have an INatsConnection interface, but extension methods (I think just request-reply methods) works with NatsConnection due to internal calls. We'd need to revisit that.

I think that they are only truly valuable as extension methods if they are extending INatsConnection. That would mean another implementer of INatsConnection would get Request/Reply extensions via the extension methods without having to do their own implementation.

As extension methods of NatsConnection they might as well be methods on NatsConnection, and members of INatsConnection

@mtmk
Copy link
Collaborator

mtmk commented Aug 4, 2023

I'm not disagreeing about having interfaces, but our clients run unit tests with actual servers. The java one starts a new server for every single test, the .NET V1 does something similar. It's imperceptibly slower. If you don't like running a server for every test, you can run an instance per suite of tests. I would say why not have a real server running since you can.

I personally prefer running my tests against the real thing in general. nats-server makes this so easy with the tiny multi-platform executable with no dependencies. That is one of NATS' great benefits in terms of testing we take it for granted.

I can also see cases when developing applications depending on NATS Client where one might want to write tests all in memory to only test the application logic for example. It might even be a personal preference depending on other factors. This doesn't necessarily help with this NATS client library development (since we are already using the server almost exclusively in our tests) but it would be useful to provide interfaces (or other utilities) to make application testing easier when using this library.

@jasper-d
Copy link
Contributor Author

jasper-d commented Aug 9, 2023

Setup a draft PR with some comments #109. There are still some things to do, but I would appreciate a quick review.

Just to clarify, this isn't meant to help NATS Client development in this repo but code that calls into it and needs to test fault handling, does not want/cannot take external test dependencies etc.

Not being able to do that was the first issue I encountered when I started to play with nats.net.v2 which I highly appreciate :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request testing
Projects
None yet
Development

No branches or pull requests

4 participants