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

Jib Builder: add more tests #2390

Merged
merged 1 commit into from Jul 2, 2019
Merged

Conversation

@dgageot
Copy link
Member

dgageot commented Jul 2, 2019

Signed-off-by: David Gageot david@gageot.net

Signed-off-by: David Gageot <david@gageot.net>
@codecov

This comment has been minimized.

Copy link

codecov bot commented Jul 2, 2019

Codecov Report

Merging #2390 into master will increase coverage by 0.2%.
The diff coverage is 100%.

Impacted Files Coverage Δ
pkg/skaffold/build/local/local.go 36.76% <ø> (ø) ⬆️
pkg/skaffold/build/local/jib_gradle.go 100% <100%> (+30.43%) ⬆️
pkg/skaffold/build/local/jib_maven.go 100% <100%> (+22.22%) ⬆️
@@ -35,6 +35,11 @@ import (
"github.com/sirupsen/logrus"
)

// For testing
var (

This comment has been minimized.

Copy link
@loosebazooka

loosebazooka Jul 2, 2019

Member

I don't really understand this specific part? Does this just become a variable in the scope of the local pkg? Was there some problem with using the direct reference (docker.RemoteDigest) in the jib files? Mostly because it seems harder to find the reference now, when looking through code.

This comment has been minimized.

Copy link
@dgageot

dgageot Jul 2, 2019

Author Member

This is the simplest way to mock the call to docker.RemoteDigest. We use that pattern in. a lot of places.

@dgageot

This comment has been minimized.

Copy link
Member Author

dgageot commented Jul 2, 2019

@loosebazooka Looks like the changes to the owners file are still not enough

@dgageot dgageot merged commit 36edb70 into GoogleContainerTools:master Jul 2, 2019
3 checks passed
3 checks passed
cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
kokoro CI build successful.
Details
@TadCordle

This comment has been minimized.

Copy link
Member

TadCordle commented Jul 2, 2019

Might be because you made a change to local.go?

@loosebazooka

This comment has been minimized.

Copy link
Member

loosebazooka commented Jul 2, 2019

@dgageot as a followup perhaps change all of the docker.RemoteDigest -> getRemoteDigest ?

build/local/bazel.go:90: return docker.RemoteDigest(tag, insecureRegistries)
build/local/custom.go:38: return docker.RemoteDigest(tag, b.insecureRegistries)
build/cache/save.go:110: return docker.RemoteDigest(img, c.insecureRegistries)
build/cache/save.go:114: return docker.RemoteDigest(img, c.insecureRegistries)
build/cache/cache.go:58: remoteDigest    = docker.RemoteDigest
build/cluster/kaniko.go:91: return docker.RemoteDigest(tag, b.insecureRegistries)
build/cluster/cluster.go:71: return docker.RemoteDigest(tag, b.insecureRegistries)

@dgageot

This comment has been minimized.

Copy link
Member Author

dgageot commented Jul 2, 2019

Well, let’s do it only when a test requires it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.