search

Azure / azure-service-bus-dotnet

☁️ .NET Standard client library for Azure Service Bus
https://azure.microsoft.com/services/service-bus
Other
235 stars 120 forks source link

classes Message and SystemPropertiesCollection must provide abstractions to allow testing #691

Open ApolloCreek opened 5 years ago

ApolloCreek commented 5 years ago

issue:

Message has no abstractions (either interface or abstract base class), but it has 'internal' setter and uses the sealed (why the heck is this?!) SystemPropertiesCollection class. This completely avoids these classes being mocked and used in unit or behavior testing (while this would easily be achievable with proper bases)!

example:

All message handler testing is impossible, at least because properties are inaccessible. E.g. depending on message.SystemProperties.LockToken results in an InvalidOperationException: code with await messageReceiver.DeadLetterAsync(message.SystemProperties.LockToken); or alike under test inevitably crashes because message cannot be faked for that use! Similar cases for several properties of Message appear all the way through tests of message consuming handlers.

solution:

Provide abstractions, make these accessible and base your implementation on those!

axisc commented 5 years ago

@ApolloCreek Both Message and SystemPropertiesCollection are data objects so interface would not be helpful here.

I agree with you that the 'internal' setter is not the best idea and we would look to remove this in the future releases.

If you'd like, please feel free to submit a PR on this, but in the new repo

SeanFeldman commented 5 years ago

@axisc I believe as a repo owner you have the ability to transfer issues between repositories.