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

update nock from 10.0.6 to 11.3.2 #4992

Merged
merged 1 commit into from
Sep 5, 2019

Conversation

KarishmaGhiya
Copy link
Member

No description provided.

@mikeharder
Copy link
Member

From the nock release notes:

For many developers no code changes will be needed. However, there are several
minor changes to the API, and it's possible that you will need to update your
code for Nock to keep working properly. It's unlikely that your tests will
falsely pass; what's more probable is that your tests will fail until the
necessary changes are made.

If all the unit tests (playback mode) pass, we should try to update recordings for at least one library as well to ensure the recording side of nock still works.

CC: @HarshaNalluru

@ramya-rao-a
Copy link
Contributor

cc @sadasant and @jonathandturner

@@ -116,7 +116,7 @@
"mocha-junit-reporter": "^1.18.0",
"mocha-multi": "^1.1.3",
"nise": "^1.4.10",
"nock": "^10.0.6",
"nock": "^11.3.2",
Copy link
Member

@HarshaNalluru HarshaNalluru Sep 5, 2019

Choose a reason for hiding this comment

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

@sadasant
@azure/test-utils-recorder is imported by keyvault-secrets, right ?
Why is keyvault-secrets still depending on nock ?

Copy link
Member

Choose a reason for hiding this comment

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

If we want to change this, please do it in a separate PR (either before or after this PR is merged)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure we don't need nock here. I most certainly just forgot to remove it. Can we remove it as part of this PR?

Copy link
Member

Choose a reason for hiding this comment

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

I'd still rather do this in a separate PR. I want to keep the version update PRs pure.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good! Here's an issue: #5003

@KarishmaGhiya
Copy link
Member Author

Tested the recording tests for node for storage-blob. They succeed as expected
Browser tests are not affected by nock

@KarishmaGhiya KarishmaGhiya merged commit 8c0cd3e into Azure:master Sep 5, 2019
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.

5 participants