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

Data collection rules 2019-11-01-preview #15789

Merged
merged 7 commits into from
Oct 14, 2020
Merged

Data collection rules 2019-11-01-preview #15789

merged 7 commits into from
Oct 14, 2020

Conversation

herreraj-ms
Copy link
Contributor

[Monitor] Added DataCollectionRules in 2019-11-01-preview

#region DCR Tests
[Fact]
[Trait("Category", "Mock")]
public void CreateDataCollectionRuleTest()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these tests mocked rather than using the test recording mechanism?

Copy link
Contributor Author

@herreraj-ms herreraj-ms Oct 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the instructions, is mention "Add or update tests for the newly generated code" but it's not specific.
I took an example of another Test and adjust accordingly.
Should I remove this file?
For the recording mechanism, Is the json file created manually? or there's a tool/procedure for creating it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The advantage of the recording mechanism is that it records responses during live tests with the service, and whenever the api-version changes, the recording is invalidated and you have to re-run the tests live. In this case, maintainers of the test will have to compare the mocked responses with service responses each time there is a chance, which increases the chance of confusion and errors.

It is appropriate to use fully mocked responses like this to simulate responses that are difficult to get from the live service (for example, to siulate handling of 409 errors). If you think this is such a case, where the service responses you are lookign for would be difficult to gt from a live test, then using this kind of mocking is appropriate. I know that there are some monitor tests (particularly around alerts) that are very difficult to simulate in live tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markcowl Recorded Test Session added!

@@ -6,11 +6,12 @@
<PropertyGroup>
<Description>Microsoft Azure Monitor Library</Description>
<AssemblyName>Microsoft.Azure.Management.Monitor</AssemblyName>
<Version>0.25.2-preview</Version>
<Version>0.25.3-preview</Version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we ever going to get to 1.0? Just asking. There are large blockers to many customers using pre-release SDKs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure who it's the appropriate person to answer this.
In the case of DataCollectionRules, it's a feature of a larger product (Azure Monitor)

@markcowl markcowl added the needs-author-feedback Workflow: More information is needed from author to address the issue. label Oct 9, 2020
@markcowl markcowl added needs-review and removed needs-author-feedback Workflow: More information is needed from author to address the issue. needs-revision labels Oct 13, 2020
@markcowl markcowl merged commit 7eaa9bc into Azure:master Oct 14, 2020
annelo-msft pushed a commit to annelo-msft/azure-sdk-for-net that referenced this pull request Feb 17, 2021
* Autorest codeGen; Removing Error*.cs models

* Autorest DCR adjust: Set apiVersion to 219-11-01-preview in DCR

* DCR & DCRA

* Adding DCRA & Testing

* Update on csproj & RP.props file

* Update monitor_resource-manager.txt & SdkInfo changes

* Fix Creadential in DCR & DCRA service operation API; Add Recorded Session Test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants