[MetricsAdvisor] Made NotificationHook abstract #22345
Conversation
kinelski
added
the
Client
ghost
added
the
sdk/metricsadvisor/Azure.AI.MetricsAdvisor/src/Models/NotificationHook/NotificationHook.cs
Show resolved
Hide resolved
| public SubstringConstraint ContainsJsonString(string propertyName, string propertyValue) => | ||
| Contains.Substring($"\"{propertyName}\":\"{propertyValue}\""); | ||
|
|
||
| // Currently only supports a single-element array. | ||
| public SubstringConstraint ContainsJsonStringArray(string propertyName, string elementValue) => | ||
| Contains.Substring($"\"{propertyName}\":[\"{elementValue}\"]"); |
There was a problem hiding this comment.
Convenience methods to make Asserts prettier (examples in tests).
sdk/metricsadvisor/Azure.AI.MetricsAdvisor/tests/Models/NotificationHookModelTests.cs
Show resolved
Hide resolved
sdk/metricsadvisor/Azure.AI.MetricsAdvisor/tests/Models/NotificationHookModelTests.cs
Show resolved
Hide resolved
kinelski
requested review from
maririos
and removed request for
tg-msft and
KrzysztofCwalina
| MockRequest request = mockTransport.Requests.Last(); | ||
| string content = ReadContent(request); | ||
|
|
||
| Assert.That(request.Uri.Path, Is.EqualTo($"/metricsadvisor/v1.0/hooks/{FakeGuid}")); |
There was a problem hiding this comment.
do you really need to validate the uri? I didn't see anything in your code that will modify this.
Also, consider making the version match the version the user sets in the client so you don't have to update this test every time you update the service version.
There was a problem hiding this comment.
We're checking that we could get and send back every NotificationHook property. We're doing it for the URI as well because that's the only way of validating the Id property (we don't send it as part of the JSON body).
You have a point about the version. I'll update the assertion to a Contains.Substring(FakeGuid) given that we just want to test if the ID is being sent correctly.
Making
NotificationHookabstract after architect feedback.Similar changes are expected for
MetricFeedback,DataFeedSource, andDataSourceCredentialEntity.