[Service Bus Client] Initial Live Test Infrastructure

[jsquire]

Summary

The focus of these changes is to introduce infrastructure to allow the Service Bus live tests to dynamically manage resources, creating a namespace for each test run and allowing each test to request a dedicated queue or topic scope.

Tests using this infrastructure are safe to run in parallel and isolated such that tests are using individual Azure resources and will not cascade failures nor should one test be able to impact the operation of another.

Last Upstream Rebase

Wednesday, February 12, 8:42am (EST)

[jsquire]

This has ties to the HTTP test recording framework beyond applying the category; since there's no equivalent recording for AMQP and Service Bus is explicitly opting-out of the recording framework, I felt it was better to convert this over to a pure set of category metadata.

[jsquire]

I believe these are more appropriate via containment, as part of a test utility class rather than using an inheritance chain to share functionality. Generally, that's considered an anti-pattern in OO. That said, I'm going to leave them as-is for now to try and avoid orthogonal changes. I'm keeping a note for a future refactoring pass when time allows.

[AlexGhiondea]

I would like to get this to follow the stablished patterns

[jsquire]

I would like to get this to follow the stablished patterns

@AlexGhiondea : Per our offline conversation, I've removed the assets that were viewed as causing friction with the engineering system approach and will reintroduce in a separate PR. Would appreciate if you'd consider unblocking.

[JoshLove-msft]

nit: should mention topic

Any chance this and the queueScope code can be combined - seems the only difference is the name of the property on the management client.

[jsquire]

gah... yeah. Queue is a typo. The split allows you to access members specific to the type of scope that you're using as well as keeping the cleanup logic focused. The counter-question that I'd pose is "what do you feel the benefit of having a meta class that combines two different entities would be?"

[JoshLove-msft]

It would be nice to avoid duplicating code so if things change we only have to update one spot.
Also Queues and Topics really are both ServiceBusEntities so having an abstract class ServiceBusEntity that they derive from makes sense to me.

[jsquire]

What would you change, though, that is shared? The dispose method name can't change and the implementation is entity specific. The NamespaceName and ShouldRemoveAtScopeCompletion are really the only shared members that you could change, and both are simple property getters. I'm going to take a YANGI approach here; if we find that we have common members that we want to share amongst entities and would have to add them multiple times, I think that's the point of reconsidering whether a base entity scope makes sense.

[JoshLove-msft]

The DisposeAsync implementations look identical except for the name of the property that holds the entity name and the type of the entity on the ServiceBusManagementClient. If you had an EntityName property on an abstract class I would think the entire method could be moved up one level and shared by both TopicScope and QueueScope. You would still have Delete methods on TopicScope/QueueScope that the abstract class DisposeAsync method would delegate to for that one line - await CreateRetryPolicy().ExecuteAsync(() => client.Queues.DeleteAsync(resourceGroup, NamespaceName, QueueName)).ConfigureAwait(false);

[jsquire]

Fair, and a legit observation. I think the point where we disagree is whether taking on additional complexity by introducing a class hierarchy offers greater benefit by removing duplication than it does hurting locality of information and creating a greater cognitive burden on those reading the code. I find myself in the camp that believes we often are premature to introduce complexity. In this particular case, I'm of the opinion that its worth trading that small bit of duplication of sparse, and simple boilerplate code that we're unlikely to revisit for the reduced complexity ability to understand at-a-glance.

[JoshLove-msft]

If we need the different types for queues vs topics, can we inherit from abstract class (or delegate to utility class) to avoid some of the property and method duplication.

[jsquire]

I don't see a benefit to using a hierarchy for something lightweight as this where only two parameters are really shared. The only scenario that I can think of where you'd want to take a couple of scopes and treat them equally would be something like "I had a test with queues and topics and want to just dispose them all at once" which the interface supports - but is quite unlikely due to using syntax.

[jsquire]

Now that the T2 Service Bus code is in master, closing this out and moving these changes to master as well.