test: lint and add tests for helmfile move#853
test: lint and add tests for helmfile move#853ankitm123 wants to merge 1 commit intojenkins-x-plugins:mainfrom
Conversation
Signed-off-by: ankitm123 <ankitmohapatra123@gmail.com>
|
Kudos, SonarCloud Quality Gate passed!
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| func (o *Options) moveFilesToClusterOrNamespacesFolder(dir, ns, releaseName, chartName string) error { | ||
| var err error | ||
| o.ClusterWide = make(map[string]bool) | ||
| JXClient, err := jxclient.LazyCreateJXClient(nil) |
There was a problem hiding this comment.
Do we need jxclient? Unit tests pass with normal kubeclient, but I am not sure ...
| } | ||
|
|
||
| func (o *Options) isClusterWide(kind string, apiVersion string, client versioned.Interface) (bool, error) { | ||
| if kube.IsNoKubernetes() { |
There was a problem hiding this comment.
Do we need this anymore?
|
Also I don't what is so bad in needing JX_NO_KUBERNETES=true, where there is already KUBECONFIG=/cluster/connections/not/allowed. |
There were some linting issues as well, which were seen in another PR, I am ok closing this one (let's just copy over the tests and lint fixes)
I dont see the point in using this variable for unit tests. With |
I think from our point of view, we should be using our own complete pipelines. But if someone wants to use jx version in their github action then I don't see why we should prevent them from doing that. It's nice to have the optionality, but it also greatly improves complexity. |
In that case, we should have 2 test cases - one with JX_NO_KUBERNETES and one where we test the kubernetes part (using the fake client) |
Actually this PR demonstrates that it is not easy to test, since the test passes while in reality the code failed in the bdd tests. |
That is because we only have one test case - only test the happy path (hence in draft mode) - I would like to replicate the bdd test failure using unit tests tbh (I will search the logs). Other reasons could be the test data is not sufficient. I firmly we believe our testing (unit and integration) will improve with time :) |
4 participants
Signed-off-by: ankitm123 ankitmohapatra123@gmail.com