-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 more kubectx tests #111
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly good! Thanks a lot.
test/common.bash
Outdated
teardown() { | ||
rm -f $KUBECONFIG | ||
rm -f $XDG_CACHE_HOME/kubectx | ||
rmdir $XDG_CACHE_HOME |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can combine all these into one rm -rf $XDG_CACHE_HOME
, if you put kubeconfig in that dir, too.
test/common.bash
Outdated
|
||
# bats setup function | ||
setup() { | ||
export KUBECONFIG=$(mktemp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why you don't do this like
KUBECONFIG="$XDG_CACHE_HOME/kubeconfig"
you're gonna get a random dir every time anyway.
test/common.bash
Outdated
# wrappers around "kubectl config" command | ||
|
||
add_cluster() { | ||
kubectl config set-cluster ${1} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider putting "" around all variable references, this prevents splitting by space.
test/kubectx.bats
Outdated
} | ||
|
||
@test "create two contexts and switch between contexts" { | ||
add_cluster cluster1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like we are gonna repeat this 2-context kubeconfig file a lot. I wish we had a way of automatically getting that.
test/kubectx.bats
Outdated
add_cluster cluster1 | ||
add_user user1 | ||
add_user user2 | ||
add_context user1@cluster1 user1 cluster1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you should modify $KUBECONFIG file in tests.
You can consider doing a cp testdata/kubeconfig "$(mktemp)"
for each test, and use that copy. Otherwise we'd be mutating kubeconfig, which can cause problems when we reorder tests. Ideally each test should be idempotent/isolated.
Looking good. Thanks! 🙌 |
No description provided.