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

[KeyVault-Keys] Missing tests and a tweak on the recordings. #6126

Merged
merged 3 commits into from
Nov 11, 2019

Conversation

sadasant
Copy link
Contributor

@sadasant sadasant commented Nov 8, 2019

Added missing tests for importKey, getDeletedKey and purgeDeletedKey. I also changed some previously unchanged before/after calls to beforeEach and afterEach, since that makes future diffs smaller and future new recordings easier.

The import code comes from: https://github.com/Azure/azure-sdk-for-python/blob/1d42b625af9d66cc94fd87bad383464c0f6ade4c/sdk/keyvault/azure-keyvault-keys/tests/test_key_client.py#L111

Fixes #5731

@sadasant sadasant requested a review from sophiajt November 8, 2019 21:17
@sadasant sadasant self-assigned this Nov 8, 2019
@@ -463,7 +463,7 @@ export class KeyClient {
public async importKey(
name: string,
key: JsonWebKey,
options: ImportKeyOptions
options?: ImportKeyOptions
Copy link
Contributor

Choose a reason for hiding this comment

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

This was originally made required because of aligning with the C# change:

"Remove ImportKey overload that does NOT take ImportKeyOptions"

I'm fine making it optional, but was trying to align with the change request.

Copy link
Contributor

Choose a reason for hiding this comment

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

On second thought, everything in ImportKeyOptions is optional, so it doesn't make sense to require this.

@sadasant sadasant merged commit 8484e25 into Azure:master Nov 11, 2019
@sadasant sadasant deleted the feature/fix5731 branch November 11, 2019 20:10
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.

[keyvault-keys] Add a test for importKey
2 participants