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

Bump moby/moby vendoring and fix tests #1504

Merged
merged 2 commits into from
Nov 8, 2018

Conversation

simonferquel
Copy link
Contributor

A recent change in moby/moby (moby/moby@3e5b9cb#diff-ac3eb5494dc3178ea5947441c086942b) made tests with missing client function mocks fail with a panic (underlying http client is nil within unit tests). This PR bumps Moby/moby and fix the impacted tests by adding missing mock functions.

- What I did

  • Bumped moby/moby
  • Fixed panicking tests

- How I did it

  • Added missing mocks

- How to verify it
Run unit tests

@simonferquel
Copy link
Contributor Author

cc @cpuguy83 @AkihiroSuda

Copy link
Collaborator

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM if green

Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

?? vendor/github.com/docker/docker/contrib/
?? vendor/golang.org/x/crypto/otr/
?? vendor/golang.org/x/crypto/ssh/test/
?? vendor/golang.org/x/sys/cpu/
?? vendor/golang.org/x/sys/windows/svc/
?? vendor/k8s.io/kubernetes/build/

validation of the vendor fails, otherwise 👍

A recent change in moby/moby made tests with missing client mocks fail with panic.
This adds those missing mocks for the impacted tests.

Signed-off-by: Simon Ferquel <[email protected]>
@codecov-io
Copy link

Codecov Report

Merging #1504 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1504   +/-   ##
=======================================
  Coverage   55.14%   55.14%           
=======================================
  Files         289      289           
  Lines       19371    19371           
=======================================
  Hits        10683    10683           
+ Misses       7997     7996    -1     
- Partials      691      692    +1

@simonferquel
Copy link
Contributor Author

@vdemeester It was a mismatch of vndr version between my dev env and the CLI dev image. Fixed!

Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐯

Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

@silvin-lubecki silvin-lubecki merged commit 3a6f8b6 into docker:master Nov 8, 2018
@GordonTheTurtle GordonTheTurtle added this to the 19.03.0 milestone Nov 8, 2018
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.

6 participants