Skip to content

[Recorder] Fixing the missed await for recorder.stop() in the tests in all the packages 📝#12062

Merged
HarshaNalluru merged 3 commits intoAzure:masterfrom
HarshaNalluru:harshan/recorder/await-stop
Oct 28, 2020
Merged

[Recorder] Fixing the missed await for recorder.stop() in the tests in all the packages 📝#12062
HarshaNalluru merged 3 commits intoAzure:masterfrom
HarshaNalluru:harshan/recorder/await-stop

Conversation

@HarshaNalluru
Copy link
Copy Markdown
Contributor

@HarshaNalluru HarshaNalluru commented Oct 26, 2020

#10013 updated the stop() method that is exported with the recorder to return a promise to make sure certain heavy recordings were recorded properly in the browser, @jeremymeng had updated all the packages to await recorder.stop().
However, some of the tests missed this update. This PR attempts to fix the rest of the tests and any of the md files that are outdated w.r.t this change.

Please make sure the call is awaited from now on. Thanks to @willmtemple for pointing out!

@ghost ghost added KeyVault Storage Storage Service (Queues, Blobs, Files) Cognitive - Form Recognizer labels Oct 26, 2020
Copy link
Copy Markdown
Contributor

@sadasant sadasant left a comment

Choose a reason for hiding this comment

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

Thank you, Harsha!

Copy link
Copy Markdown

@mge mge left a comment

Choose a reason for hiding this comment

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

EventGrid stuff looks good to me. Thanks for going ahead and cleaning this up!

Copy link
Copy Markdown
Member

@ellismg ellismg left a comment

Choose a reason for hiding this comment

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

From the correct account this time :-)

EventGrid stuff looks good to me. Thanks for going ahead and cleaning this up!

Copy link
Copy Markdown
Member

@jeremymeng jeremymeng left a comment

Choose a reason for hiding this comment

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

Looks good! Do we want a linter rule for this specific case?

@HarshaNalluru
Copy link
Copy Markdown
Contributor Author

Do we want a linter rule for this specific case?

Seems like an overkill. Should be good enough to update the docs and existing tests.

@HarshaNalluru HarshaNalluru merged commit ed9587d into Azure:master Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Cognitive - Form Recognizer KeyVault Storage Storage Service (Queues, Blobs, Files)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants