Add management connection string API and tests #1519
Conversation
TravisNickels
force-pushed
the
travis/management-connection
branch
from
78161c8 to
881bedf
Compare
TravisNickels
changed the title
src/NServiceBus.Transport.RabbitMQ/Administration/ManagementClient/ManagementClient.cs
Outdated
Show resolved
Hide resolved
src/NServiceBus.Transport.RabbitMQ/Administration/ManagementClient/ManagementClient.cs
Outdated
Show resolved
Hide resolved
src/NServiceBus.Transport.RabbitMQ/Administration/ManagementClient/ManagementClient.cs
Outdated
Show resolved
Hide resolved
src/NServiceBus.Transport.RabbitMQ/Administration/ManagementClient/ManagementClient.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Adding obsolete methods to a new major version feels a bit wrong to me, but I don't know the full context around the API mismatch.
There was a problem hiding this comment.
I wasn't really sure about this either. However, my thought process on this was that if users are still able to use the other obsolete methods for connectionString, I figured it would make sense to add the managementConnectionString method as well so it would be easier to integrate with their existing application and when the obsoletes are actually removed the migration to the updated API can all be done at once.
| } | ||
|
|
||
| [Test] | ||
| public void Should_configure_broker_and_management_connection_configurations_with_single_connection_string() |
There was a problem hiding this comment.
Since all the following tests are actually connecting to RabbitMQ, I'm wondering if they should be moved to the acceptance tests assembly.
There was a problem hiding this comment.
Yeah, I wasn't 100% sure about this either. However, the RabbitMQContext.cs also connects to RabbitMQ, but it looks like only the tests that start with When_ are inheriting RabbitMQContext. which seem to all be acceptance tests, I think? However, they are still in the same Tests project. I could separate them into a separate file that starts with When_ to indicate that they are acceptance tests. I don't know if that is a standard naming convention for acceptances tests, but it seems right.
src/NServiceBus.Transport.RabbitMQ/Administration/ManagementClient/ManagementClient.cs
Outdated
Show resolved
Hide resolved
No description provided.