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

Monitor Fix Missing ResourceId Bug #44186

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jairmyree
Copy link
Member

Bug fix for #43841

@jairmyree jairmyree requested review from srnagar, lmolkova and a team as code owners February 13, 2025 20:35
@github-actions github-actions bot added the Monitor Monitor, Monitor Ingestion, Monitor Query label Feb 13, 2025
@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

}

@Test
public void testQueryResourcesReturnsNonNullResourceId() {
Copy link
Member

Choose a reason for hiding this comment

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

We don't need a separate test to check if resourceId is not null. We can just add an assertEquals to the previous test.

@@ -75,4 +75,33 @@ public void testMetricsBatchQueryDifferentResourceTypes() {

}

@Test
public void testQueryResourcesReturnsNonNullResourceId() {
Copy link
Member

Choose a reason for hiding this comment

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

Same here - we can just update the previous test to check for resource id in the response.

= metricsClient
.queryResourcesWithResponse(Arrays.asList(resourceId), Arrays.asList("HttpIncomingRequestCount"),
"microsoft.appconfiguration/configurationstores", options)
.block()
Copy link
Member

Choose a reason for hiding this comment

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

Async tests should not use block(). Use StepVerifier instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Monitor Monitor, Monitor Ingestion, Monitor Query
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants