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

Sleep for 1.5 seconds before looking at the resize error #3584

Merged
merged 1 commit into from
May 3, 2022

Conversation

rumpl
Copy link
Member

@rumpl rumpl commented May 2, 2022

- What I did

Sleep for 1.5 seconds before looking at the resize error

This test is very flaky, the retry loop runs for 550ms and some more, 750ms is cleary not enough for everything to set and for the cli to return the tty resize error. A sleep of 1.5 seconds in this test should be enough for the retry loop to finish.

Relates to #3554

- How I did it

- How to verify it

Run the CI a lot of times

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)
image

This test is very flaky, the retry loop runs for 550ms and some more, 750ms is
cleary not enough for everything to set and for the cli to return the tty resize
error. A sleep of 1.5 seconds in this test should be enough for the retry loop to
finish.

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
@codecov-commenter
Copy link

Codecov Report

Merging #3584 (0c2d007) into master (c76cbdc) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3584   +/-   ##
=======================================
  Coverage   59.46%   59.47%           
=======================================
  Files         285      287    +2     
  Lines       24171    24174    +3     
=======================================
+ Hits        14373    14377    +4     
  Misses       8931     8931           
+ Partials      867      866    -1     

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

let me know when it's ready to be merged (you mentioned wanting to run CI a couple of times)

@rumpl
Copy link
Member Author

rumpl commented May 3, 2022

OK I've ran the CI 5-6 times yesterday and 4 times today, I think this can be merged.

@thaJeztah
Copy link
Member

whoop; let's get it in 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants