Update function signature of testutil.NewGenericTestHelper
#4591
Conversation
| @@ -76,9 +77,11 @@ func NewGenericTestHelper( | |||
| helmReleaseName string, | |||
| externalIssuer, | |||
| uninstall bool, | |||
| httpClient http.Client, | |||
| ) *TestHelper { | |||
| return &TestHelper{ | |||
There was a problem hiding this comment.
Looks like there is also KubernetesHelper variable in TestHelper. Maybe we can use that somewhere in the code using the returned value from NewKubernetesHelper()?
There was a problem hiding this comment.
Ah, nice catch - was only considering setting the private fields through this method, but it certainly makes sense to initialize the KubernetesHelper field as well. That would mean that NewGenericTestHelper would now also have to accept k8sContext as a parameter and also return the error returned from NewKubernetesHelper.
I have pushed the changes incorporating the above. PTAL! 😄
|
@mayankshah1607 Would you mind updating the PR title to describe what this change does? Consider how this description will fit in the output of |
mayankshah1607
changed the title
testutil.NewGenericTestHelper
testutil/test_helper.go
Outdated
| } | ||
|
|
||
| kubernetesHelper, err := NewKubernetesHelper(k8sContext, h.RetryFor) |
There was a problem hiding this comment.
As the kubernetesHelper is public, and so is NewKubernetesHelper(), why can't we just take it as a argument and assign it directly, rather than using this function directly. Any upsides to this?
mayankshah1607
force-pushed
the
mayank/testutil-followup
branch
from
c39849c to
e02bec8
Compare
- Updates function signature of `NewGenericTestHelper` to accept `version` and an `httpClient` Signed-off-by: Mayank Shah <mayankshah1614@gmail.com>
- accept `k8sContext` as a parameter for the above method - set the `KubernetesHelper` field in the returned `TestHelper` - return errors encountered in the function body Signed-off-by: Mayank Shah <mayankshah1614@gmail.com>
- remove `k8sContext` as parameter - add `kubernetesHelper` as parameter - initialize KubernetesHelper field directly using the `kubernetesHelper` parameter Signed-off-by: Mayank Shah <mayankshah1614@gmail.com>
mayankshah1607
force-pushed
the
mayank/testutil-followup
branch
from
e02bec8 to
eb29dac
Compare
Signed-off-by: Mayank Shah <mayankshah1614@gmail.com>
mayankshah1607
force-pushed
the
mayank/testutil-followup
branch
from
176dbbc to
8242231
Compare
This PR is a follow-up for #4532
Once merged,
testutil.NewGenericTestHelperwillversion,httpClientandk8sContextas parametersKubernetesHelperfield in the returned instance of*TestHelperSigned-off-by: Mayank Shah mayankshah1614@gmail.com
Cc: @Pothulapati @alpeb