Skip to content

Bump moby/moby vendoring and fix tests - #1504

Merged
silvin-lubecki merged 2 commits into
docker:masterfrom
simonferquel:bump-moby-fix-tests
Nov 8, 2018
Merged

Bump moby/moby vendoring and fix tests#1504
silvin-lubecki merged 2 commits into
docker:masterfrom
simonferquel:bump-moby-fix-tests

Conversation

@simonferquel

Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

cc @cpuguy83 @AkihiroSuda

@AkihiroSuda AkihiroSuda left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, LGTM if green

@vdemeester vdemeester left a comment

Copy link
Copy Markdown
Collaborator

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 👍

Signed-off-by: Simon Ferquel <simon.ferquel@docker.com>
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 <simon.ferquel@docker.com>
@codecov-io

Copy link
Copy Markdown

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
Copy Markdown
Contributor Author

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

@vdemeester vdemeester left a comment

Copy link
Copy Markdown
Collaborator

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 left a comment

Copy link
Copy Markdown
Contributor

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