Add controller Service Instance test #1
Conversation
| defer ct.TearDown() | ||
|
|
||
| require.NoError(t, ct.CreateClusterServiceClass()) | ||
| require.NoError(t, ct.WaitForClusterServiceClass()) |
There was a problem hiding this comment.
do we need to wait for the class? the same for plan
| // in this test the `instance.Generation` is not update so in `pkg/controller/controller_instance.go` | ||
| // in method `reconcileServiceInstanceAdd` will not increase `ObservedGeneration` value | ||
| // because `instance.Status.ObservedGeneration != instance.Generation` condition is not met | ||
| require.NoError(t, ct.EnsureInstanceObservedGeneration(0)) |
| Reason: "ProvisionedSuccessfully", | ||
| } | ||
| require.NoError(t, ct.WaitForInstanceCondition(condition)) | ||
| require.NoError(t, ct.CheckFeatureGate(state.enableFeatureGate)) |
There was a problem hiding this comment.
Where do you set the feature gate?
the name CheckFeatureGate is saying - check the feature gate, but I'm looking deeper and there is a logic with dashboardURL - let's discuss structure the test
pkg/controller/case_test.go
Outdated
| } | ||
|
|
||
| // EnsureBrokerActionExist makes sure specific fake osb client action exist | ||
| func (ct *controllerTest) EnsureBrokerActionExist(actionType fakeosb.ActionType) error { |
There was a problem hiding this comment.
there are methods like: NumberOfOSBProvisionCalls which can be used
pkg/controller/case_test.go
Outdated
|
|
||
| // EnsureLastUpdateBrokerActionHasCorrectPlan makes sure osb client action with type "UpdateInstance" | ||
| // contains specific plan ID in request body parameters | ||
| func (ct *controllerTest) EnsureLastUpdateBrokerActionHasCorrectPlan(planID string) error { |
There was a problem hiding this comment.
create a method which returns last update planID or create a method something like AssertLastOSBUpdatePlanID
| defer ct.TearDown() | ||
|
|
||
| ct.SetupEmptyPlanListForOSBClient() | ||
| require.NoError(t, ct.CreateServiceBrokerWithIncreaseRelistRequests()) |
There was a problem hiding this comment.
ct.CreateSimpleClusterServiceBroker(),
why do you need increasing relist requests
| require.NoError(t, ct.WaitForServiceInstanceProcessedGeneration(generation)) | ||
|
|
||
| // THEN | ||
| require.NoError(t, ct.EnsureBrokerActionExist(fakeosb.UpdateInstance)) |
There was a problem hiding this comment.
ct.NumberOfUpdateCalls (you need to create such method, see other NumberOf... methods)
|
|
||
| // THEN | ||
| require.NoError(t, ct.EnsureBrokerActionExist(fakeosb.UpdateInstance)) | ||
| require.NoError(t, ct.EnsureBrokerActionWithParametersExist(state.expectedParams)) |
There was a problem hiding this comment.
ct.AssertLastUpdateAction, see AssertLastBindRequest
| Reason: state.orphanMitigationReason, | ||
| } | ||
| require.NoError(t, ct.EnsureServiceInstanceHasCondition(condition)) | ||
| require.NoError(t, ct.EnsureServiceInstanceOrphanMitigationStatus(true)) |
There was a problem hiding this comment.
AssertServiceinstanceOrphanMitigationStatus
| Status: v1beta1.ConditionFalse, | ||
| } | ||
| require.NoError(t, ct.EnsureServiceInstanceHasNoCondition(condition)) | ||
| require.NoError(t, ct.EnsureBrokerActionExist(fakeosb.ProvisionInstance)) |
There was a problem hiding this comment.
use ct.NumberOfProvisionCalls
pkg/controller/case_test.go
Outdated
|
|
||
| // AssertBrokerActionWithParametersExist makes sure osb client action with type "UpdateInstance" | ||
| // contains specific parameters in request body parameters | ||
| func (ct *controllerTest) AssertBrokerActionWithParametersExist(parameters map[string]interface{}) error { |
There was a problem hiding this comment.
it asserts update action - fix the name
pkg/controller/case_test.go
Outdated
|
|
||
| // AssertLastOSBUpdatePlanID makes sure osb client action with type "UpdateInstance" | ||
| // contains specific plan ID in request body parameters | ||
| func (ct *controllerTest) AssertLastOSBUpdatePlanID() error { |
There was a problem hiding this comment.
please, refactor all Assert* methods to have one argument t *testing.T, see existing AssertOSBBasicAuth
| require.NoError(t, ct.AssertServiceInstanceDashboardURL()) | ||
| } else { | ||
| require.NoError(t, ct.AssertServiceInstanceEmptyDashboardURL()) | ||
| } |
There was a problem hiding this comment.
try such test:
t.Run(tn, func(t *testing.T) {
// GIVEN
ct := newControllerTest(t)
defer ct.TearDown()
//if state.enableFeatureGate {
oldValue := utilfeature.DefaultFeatureGate.Enabled(scfeatures.UpdateDashboardURL)
utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=%v", scfeatures.UpdateDashboardURL, state.enableFeatureGate))
defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=%v", scfeatures.UpdateDashboardURL, oldValue))
//}
require.NoError(t, ct.CreateSimpleClusterServiceBroker())
require.NoError(t, ct.WaitForReadyBroker())
// WHEN
require.NoError(t, ct.CreateServiceInstance())
// THEN
require.NoError(t, ct.WaitForReadyInstance())
ct.SetUpdateServiceInstanceResponseWithDashboardURL()
ct.UpdateServiceInstanceParameters()
time.Sleep(time.Second)
if state.enableFeatureGate {
require.NoError(t, ct.AssertServiceInstanceDashboardURL())
} else {
require.NoError(t, ct.AssertServiceInstanceEmptyDashboardURL())
}
})
|
PR moved to main fork #2667 |
Related to #3113. Controller tests for ServiceInstance.