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

Increase test coverage on Jib Builder #2383

Merged
merged 1 commit into from
Jul 1, 2019

Conversation

dgageot
Copy link
Contributor

@dgageot dgageot commented Jul 1, 2019

Signed-off-by: David Gageot [email protected]

@codecov
Copy link

codecov bot commented Jul 1, 2019

Codecov Report

Merging #2383 into master will increase coverage by 0.47%.
The diff coverage is n/a.

Impacted Files Coverage Δ
pkg/skaffold/build/local/jib_maven.go 77.77% <0%> (+51.11%) ⬆️
pkg/skaffold/build/local/jib_gradle.go 69.56% <0%> (+69.56%) ⬆️

@dgageot
Copy link
Contributor Author

dgageot commented Jul 1, 2019

@dgageot dgageot added the kokoro:run runs the kokoro jobs on a PR label Jul 1, 2019
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Jul 1, 2019
{
description: "local build",
artifact: &latest.JibGradleArtifact{},
cmd: testutil.FakeRun(t, "gradle -Djib.console=plain :jibDockerBuild --image=img:tag"),
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how this testing framework works: Are we verifying that the code creates the command defined in FakeRun "gradle -Djib.console=plain :jibDockerBuild --image=img:tag"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It’s checking that command and only this one runs

Copy link
Member

Choose a reason for hiding this comment

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

kinda like a Mockito.when().then()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

@dgageot
Copy link
Contributor Author

dgageot commented Jul 1, 2019

@TadCordle should be good now. Thanks for the review!

Copy link
Member

@loosebazooka loosebazooka left a comment

Choose a reason for hiding this comment

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

Looks good, should we also be covering the remote case?

{
description: "local build",
artifact: &latest.JibGradleArtifact{},
cmd: testutil.FakeRun(t, "gradle -Djib.console=plain :jibDockerBuild --image=img:tag"),
Copy link
Member

Choose a reason for hiding this comment

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

kinda like a Mockito.when().then()?

@dgageot
Copy link
Contributor Author

dgageot commented Jul 1, 2019

Don’t worry, I’m working on the remote case but I’d rather get this merged first

@loosebazooka
Copy link
Member

Is jib team not owners?

@dgageot
Copy link
Contributor Author

dgageot commented Jul 1, 2019

That would make sense but the file pattern **/**jib** which doesn't to match jib_maven_test.go...

@dgageot dgageot merged commit e20185f into GoogleContainerTools:master Jul 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants