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-query][test] logsQueryClient unit test should not rely on ServiceClient internals #21551

Closed
jeremymeng opened this issue Apr 21, 2022 · 1 comment
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. Monitor Monitor, Monitor Ingestion, Monitor Query test-reliability Issue that causes tests to be unreliable
Milestone

Comments

@jeremymeng
Copy link
Member

Currently the test passes a custom endpoint via option then verify that private/protected properties of the generated client are set.

assert.equal(client["_logAnalytics"].$host, "https://customEndpoint1");
assert.equal(client["_logAnalytics"]["_baseUri"], "https://customEndpoint1");

We can improve the test by testing that the option is passed to ServiceClient via a mock, instead of verifying the internals of ServiceClient.

@jeremymeng jeremymeng added Client This issue points to a problem in the data-plane of the library. test-reliability Issue that causes tests to be unreliable Monitor - Log Analytics labels Apr 21, 2022
@jeremymeng jeremymeng changed the title [monitor-query] logsQueryClient unit test should not rely on ServiceClient internals [monitor-query][test] logsQueryClient unit test should not rely on ServiceClient internals Apr 21, 2022
@KarishmaGhiya KarishmaGhiya added this to the [2022] July milestone Apr 21, 2022
@scottaddie scottaddie added Monitor Monitor, Monitor Ingestion, Monitor Query Monitor - Query and removed Monitor - Log Analytics labels Jun 15, 2022
@xirzec xirzec modified the milestones: 2022-07, 2022-08 Jul 8, 2022
@xirzec xirzec modified the milestones: 2022-08, 2022-09 Aug 9, 2022
@xirzec xirzec modified the milestones: 2022-09, 2022-12 Nov 1, 2022
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-js that referenced this issue Nov 28, 2022
{AzureDataProtection} fixes Azure/azure-rest-api-specs#21183 (Azure#21551)

fixes Azure/azure-rest-api-specs#21183

The enum values for the storage settings are listed in the latest API as `ArchiveStore, SnapshotStore, VaultStore.`
However while providing `SnapshotStore` value it fail with `BMSUserErrorInvalidInput` error. Also while trying the same from PS cmdlet using

> New-AzDataProtectionBackupVaultStorageSettingObject -DataStoreType SnapshotStore -Type LocallyRedundant

**Error:**
```
Error: "Unable to match the identifier name 
SnapshotStore to a valid enumerator name. Specify one of the following enumerator names and try again:
ArchiveStore, OperationalStore, VaultStore"
```

This PR fixes the enum value of storage setting
@xirzec xirzec modified the milestones: 2022-12, 2023-02 Jan 11, 2023
@xirzec xirzec removed this from the 2023-02 milestone Mar 31, 2023
@scottaddie scottaddie added this to the 2023-07 milestone May 18, 2023
@sarangan12
Copy link
Member

Checked this code. There are 2 levels of validation that is happening in this test:

  1. Checking the Endpoints
  2. Checking the error message.

Here, if I try to check the endpoints using the mock client, then this would need the ability to modify the pipeline policy which is not currently exposed. So, doing this would be more related to a feature work for which there is no real need. So, closing this issue and keeping the tests as such. No further action required.

@github-actions github-actions bot locked and limited conversation to collaborators Sep 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. Monitor Monitor, Monitor Ingestion, Monitor Query test-reliability Issue that causes tests to be unreliable
Projects
None yet
Development

No branches or pull requests

5 participants