lack of tests for currently gardenctl

[tedteng]

Describe the bug
To make sure the code quality we need add more tests to cover the gardenctl code as much as we can to check the code quality every time when we delivery

the following exist testing which I checked only covers test invalid number args, or shoot targeted check. no function test or unit test inside. which means they are not able to check the command function itself whether working well or not.

• download_test.go

• logs_test.go

• show_test.go

• ssh_test.go

the following command no automation test case

• kubectl.go

• kubectx.go

• openstack.go

• terraform.go

• aliyun.go

• az.go

• aws.go

• gcloud.go

[neo-liang-sap]

for kubectl / kubectx / terraform i used to work on their tests and had a discussion with @dansible , for these commands we only needs to confirm the $? is 0, so we should focus on other tests esp the function we designed.
@dansible , what do you think?
Thanks.
-Neo

[tedteng]

To make sure the quality, I think we are need focus on the tests as much as we can. for the new feature defintely we need new tests to case, for the exist one which I list in the above we are missing and not fully cover. we need increase the test case to cover it. I think currently all the issue we occured caused by we are missing tests for exist code during our refactoing.

The command you mention using $? is 0 could be one of soulation for the testing, but currently kubectl / kubectx / terraform don't have any automation testing yet. We don't know if there any code impact those commands in the future unless we have testing case and check every time locally or from github when commit.

currently I am research set up aws/gcp/az cli automatical test, for the other tests I don't have solution yet. welcome any kind of feedback or code contribute.

[dansible]

@tedteng you're totally right that we need to focus on adding more comprehensive testing but I agree with @neo-liang-sap regarding the subcommands - anything that directly calls a 3rd party component just needs to be verified that it returns successfully.

kubectl / kubectx / terraform / aws / gcloud / ... all of these are external components with their own test suites. We don't need to re-test the functionality of each of these. The only thing we need to ensure is that we call these subcommands correctly with the correct arg parsing. Most of these have a --help or --version argument. This should be sufficient to verify it returns with 0. For example: gardenctl aws -- --version

[tedteng]

kubectl / kubectx / terraform / aws / gcloud / ... all of these are external components with their own test suites. We don't need to re-test the functionality of each of these. The only thing we need to ensure is that we call these subcommands correctly with the correct arg parsing. Most of these have a --help or --version argument. This should be sufficient to verify it returns with 0. For example: gardenctl aws -- --version

I think I miss the point at the beginning from neo he mentions about test kubectl/kubectx/taerraform itself. I am not talking about testing for subcommand itself, my point same as you guys we don't need test kubectl / kubectx / terraform / aws / gcloud / itself, they have their own test suits. It's not necessary for us.

the only test case we need is the subcommand which involved by gardenctl. that the function we need testing. that why I mention we don't have it. I guess that the misunderstanding here.

then I added for the test to handle this kind of test which we don't have before.

https://github.com/gardener/gardenctl/blob/dc3d2726383a451ad8f6682555c8b5c7790c55d2/.ci/test#L18-L35

[dansible]

For the temporary testing, those are good 👍 The same can apply for kubectx, terraform and kubectl

[tedteng]

For the temporary testing, those are good 👍 The same can apply for kubectx, terraform and kubectl

Thanks, @dansible . for those subcommands go files it invokes the different third commands eg (kubeclt, kubectx, terraform etc), the third commands only installed on our local machine. I guess ginkgo may not able to mock it and failure in concourse either. maybe this is the only way we can test our subcommand. welcome any new solutions.

Therefore, I suggest we create a docker image which contains those third commands, kubectl,kubectx,openstack,terraform,az,aws,gcloud, in this way we can use it in the concourse pipeline to test our subcommands whether invoke third commands successfully in the container. maybe image use in ssh test as well in the future

let me know whether on the right approach. if yes do you know which image repository I should create and use, and pipeline as well? gardener public repository image? pipeline in idefix ?

[andrei-panov]

If I understand it correctly here the image https://github.com/gardener/ops-toolbelt
Only the credentials part is missed.

[tedteng]

If I understand it correctly here the image https://github.com/gardener/ops-toolbelt
Only the credentials part is missed.

yes as ops-toolbelt is one of the options, in a container to git clone gardenctl code then test it,
as research in Github action recently, it seems more easily use an action to achieve now. so the current approach I am using action as a container to test

[tedteng]

• logs_test.go as Issue 326 sanitize logs go #425 coverd

[tedteng]

• kubectl.go
• kubectx.go
• terraform.go

will cover in #447

• ssh_test.go cover in Add script to test ssh functionality #440 and executed the test_local from github action  #447