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

Add tests for updateDependencies #1562

Merged
merged 3 commits into from Dec 17, 2018

Conversation

@mgazza
Copy link
Contributor

commented Nov 29, 2018

Added tests to ensure that this functions behaviour with helm is always defined.

Add tests for updateDependencies
Added tests to ensure that this functions behaviour with helm is always defined.
@mgazza

This comment has been minimized.

Copy link
Contributor Author

commented Nov 29, 2018

Tests pass locally. I'm not sure how to proceed it looks like helm can't update the repository.

@squaremo

This comment has been minimized.

Copy link
Member

commented Dec 11, 2018

I suspect that it's because updateDependencies relies on the helm binary, and that's not available in the CI environment. The non-existent chart test succeeds there, because it expects an error, and gets one.

Suggestion: we download the helm client as part of the build, so it could just be a dependency of make test (and added to the path).

NB when I try this locally, it's the non-existent chart test that fails, i.e., it doesn't return an error. We can see if that's the case in CI ..

@squaremo

This comment has been minimized.

Copy link
Member

commented Dec 11, 2018

Update: added grin for first-time contributor -- thanks for this PR!

squaremo added 2 commits Dec 13, 2018
Make sure the helm binary is available for tests
Some tests may depend on the helm binary, so make it a
dependency. And, since we want to test with the binary that's being
baked into the image, put it first on the $PATH.
Use a temp dir for Helm home dir
Running locally, it's likely that you'll already have helm set up; but
this isn't the case in CI, and only a coincidence. Therefore, create a
temp directory and use that as Helm's home dir when testing.
@squaremo

This comment has been minimized.

Copy link
Member

commented Dec 13, 2018

@mgazza Are you happy for me to push a couple of commits here?

@mgazza

This comment has been minimized.

Copy link
Contributor Author

commented Dec 14, 2018

Hi @squaremo,
yes please! Sorry I meant to get back to you yesterday

@squaremo

This comment has been minimized.

Copy link
Member

commented Dec 17, 2018

@hiddeco Mind having a look at this one when you have a chance? I ought to recuse myself on the basis that I've added commits :-)

@hiddeco
Copy link
Member

left a comment

LGTM! Thanks @mgazza for your initial work on this 🥇

@squaremo squaremo merged commit 7f48703 into fluxcd:master Dec 17, 2018

1 check passed

ci/circleci: build Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.