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

Implement retrying logic for VSCode E2E tests #1096

Merged
merged 2 commits into from
Dec 9, 2020

Conversation

shenglol
Copy link
Contributor

@shenglol shenglol commented Dec 9, 2020

Hopefully this can make the E2E tests pass in the ADO pipeline.

@codecov-io
Copy link

codecov-io commented Dec 9, 2020

Codecov Report

Merging #1096 (abe6db9) into main (9710a22) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1096   +/-   ##
=======================================
  Coverage   94.27%   94.27%           
=======================================
  Files         328      328           
  Lines       15856    15856           
  Branches       12       12           
=======================================
  Hits        14948    14948           
  Misses        908      908           
Flag Coverage Δ
dotnet 94.83% <ø> (ø)
typescript 25.78% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

): Promise<T> {
let result = await func();

while (predicate(result) && retryOptions.count--) {
Copy link
Member

Choose a reason for hiding this comment

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

Will the change to count propagate outside of the function call? If yes, then we should probably make a copy first. Otherwise, you could get really weird behavior if you try to reuse retry options across function calls.

Copy link
Contributor Author

@shenglol shenglol Dec 9, 2020

Choose a reason for hiding this comment

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

Oops...good catch. Never intended to write code like that. Pushed out a new commit to make that parameter read-only.

Copy link
Member

@majastrz majastrz left a comment

Choose a reason for hiding this comment

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

Signed off with a comment/question.

@shenglol shenglol merged commit c42a908 into main Dec 9, 2020
@shenglol shenglol deleted the shenglol/fix-flaky-e2e-tests branch December 9, 2020 18:04
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.

3 participants