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.

Sign up for GitHub

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

Jump to bottom

Add unit test coverage for util.RunCommand #78

Closed
r2d4 opened this issue Feb 13, 2018 · 2 comments
Closed

Add unit test coverage for util.RunCommand #78

r2d4 opened this issue Feb 13, 2018 · 2 comments
Labels
good first issue Good for newcomers help wanted We would love to have this done, but don't have the bandwidth, need help from contributors meta/testing Issues focused on testing Skaffold itself

Comments

@r2d4
Copy link
Contributor

r2d4 commented Feb 13, 2018

We should add info and debug level logging to this function also. As we add more build and deploy implementations, we'll be using this a lot so it should have good test coverage.

To test exec.Command, we can shell out to the test binary like the std library does https://github.com/golang/go/blob/master/src/os/exec/exec_test.go#L33

We can also use the fake readclosers in our testutil package.
@r2d4 r2d4 added help wanted We would love to have this done, but don't have the bandwidth, need help from contributors area/testing Issues concerning the testing phase of Skaffold labels Feb 13, 2018
@yakovzaytsev
Copy link
Contributor

yakovzaytsev commented Mar 11, 2018 •
edited
Loading

All util.RunCommand as of 9c08383 does is call out to Commander.RunCommand Should I add tests to that one?

@r2d4
Copy link
Contributor Author

r2d4 commented Mar 12, 2018

Yep, thats the one I was referring to. That would be amazing!

@r2d4 r2d4 added the good first issue Good for newcomers label Apr 11, 2018
@taylorKonigsmark taylorKonigsmark mentioned this issue Jun 28, 2018
Merged
@r2d4 r2d4 closed this as completed Aug 15, 2018
@nkubala nkubala added meta/testing Issues focused on testing Skaffold itself and removed area/testing Issues concerning the testing phase of Skaffold labels Sep 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers help wanted We would love to have this done, but don't have the bandwidth, need help from contributors meta/testing Issues focused on testing Skaffold itself
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@r2d4 @nkubala @yakovzaytsev