Skip to content

Commit

Permalink
simplfy test
Browse files Browse the repository at this point in the history
  • Loading branch information
@asalan316
asalan316 committed Nov 28, 2024
1 parent 23e84f2 commit 351b1ce
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 123 deletions.
73 changes: 9 additions & 64 deletions src/acceptance/api/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,15 @@ var _ = Describe("AutoScaler Public API", func() {
})

It("should fail to create an invalid custom metrics submission", func() {
By("creating custom metrics submission with invalid string")
response, status := createPolicy(GenerateBindingsWithScalingPolicy("invalid-value", 1, 2, "memoryused", 30, 100))
Expect(string(response)).Should(ContainSubstring(`configuration.custom_metrics.metric_submission_strategy.allow_from must be one of the following: \"bound_app\", \"same_app\""}`))
Expect(string(response)).Should(MatchJSON(`[{"context":"(root).configuration.custom_metrics.metric_submission_strategy.allow_from","description":"configuration.custom_metrics.metric_submission_strategy.allow_from must be one of the following: \"bound_app\""}]`))
Expect(status).To(Equal(400))

By("creating custom metrics submission with empty value ' '")
policy := GenerateBindingsWithScalingPolicy("", 1, 2, "memoryused", 30, 100)
newPolicy, status := createPolicy(policy)
Expect(string(newPolicy)).Should(MatchJSON(`[{"context":"(root).configuration.custom_metrics.metric_submission_strategy.allow_from","description":"configuration.custom_metrics.metric_submission_strategy.allow_from must be one of the following: \"bound_app\""}]`))
Expect(status).To(Equal(400))
})

Expand All @@ -124,68 +131,6 @@ var _ = Describe("AutoScaler Public API", func() {
response, status := createPolicy(policy)
Expect(string(response)).Should(MatchJSON(policy))
Expect(status).To(Or(Equal(200), Equal(201)))
/**
STEP: PUT 'https://autoscaler-3317.autoscaler.app-runtime-interfaces.ci.cloudfoundry.org/v1/apps/17ae5290-c63a-4836-81a6-42d9635c293a/policy' - /Users/I545443/SAPDevelop/asalan316/app-autoscaler-release/src/acceptance/api/api_test.go:308 @ 11/18/24 13:53:22.373
[FAILED] Expected
<string>: {
"instance_min_count": 1,
"instance_max_count": 2,
"scaling_rules": [
{
"metric_type": "memoryused",
"breach_duration_secs": 60,
"threshold": 100,
"operator": "\u003e=",
"cool_down_secs": 60,
"adjustment": "+1"
},
{
"metric_type": "memoryused",
"breach_duration_secs": 60,
"threshold": 30,
"operator": "\u003c",
"cool_down_secs": 60,
"adjustment": "-1"
}
]
}
to match JSON of
<string>: {
"configuration": {
"custom_metrics": {
"metric_submission_strategy": {
"allow_from": ""
}
}
},
"instance_min_count": 1,
"instance_max_count": 2,
"scaling_rules": [
{
"metric_type": "memoryused",
"breach_duration_secs": 60,
"threshold": 100,
"operator": ">=",
"cool_down_secs": 60,
"adjustment": "+1"
},
{
"metric_type": "memoryused",
"breach_duration_secs": 60,
"threshold": 30,
"operator": "<",
"cool_down_secs": 60,
"adjustment": "-1"
}
]
}
By("creating custom metrics submission with empty value ' '")
policy = GenerateBindingsWithScalingPolicy("", 1, 2, "memoryused", 30, 100)
response, status = createPolicy(policy)
Expect(string(response)).Should(MatchJSON(policy))
Expect(status).To(Or(Equal(200), Equal(201)))*/
})

})
Expand Down Expand Up @@ -316,7 +261,7 @@ var _ = Describe("AutoScaler Public API", func() {
It("should fail to update an invalid custom metrics strategy", func() {
expectedPolicy = GenerateBindingsWithScalingPolicy("invalid-update", 1, 2, "memoryused", 30, 100)
actualPolicy, status = createPolicy(expectedPolicy)
Expect(string(actualPolicy)).Should(ContainSubstring(`configuration.custom_metrics.metric_submission_strategy.allow_from must be one of the following: \"bound_app\", \"same_app\""}`))
Expect(string(actualPolicy)).Should(MatchJSON(`[{"context":"(root).configuration.custom_metrics.metric_submission_strategy.allow_from","description":"configuration.custom_metrics.metric_submission_strategy.allow_from must be one of the following: \"bound_app\""}]`))
Expect(status).To(Equal(400))
})
It("should succeed to update a valid custom metrics strategy", func() {
Expand Down
29 changes: 10 additions & 19 deletions src/autoscaler/api/publicapiserver/public_api_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import (
"os"
"reflect"

"github.com/pivotal-cf/brokerapi/v11/domain/apiresponses"

"code.cloudfoundry.org/app-autoscaler/src/autoscaler/api/config"
"code.cloudfoundry.org/app-autoscaler/src/autoscaler/api/policyvalidator"
"code.cloudfoundry.org/app-autoscaler/src/autoscaler/api/schedulerclient"
Expand Down Expand Up @@ -43,10 +41,7 @@ const (
ErrorMessageAppidIsRequired = "AppId is required"
)

var (
ErrInvalidConfigurations = errors.New("invalid binding configurations provided")
ErrInvalidCustomMetricsStrategy = errors.New("error: custom metrics strategy not supported")
)
var ErrInvalidConfigurations = errors.New("invalid binding configurations provided")

func NewPublicApiHandler(logger lager.Logger, conf *config.Config, policydb db.PolicyDB, bindingdb db.BindingDB, credentials cred_helper.Credentials) *PublicApiHandler {
egClient, err := helpers.CreateHTTPSClient(&conf.EventGenerator.TLSClientCerts, helpers.DefaultClientConfig(), logger.Session("event_client"))
Expand Down Expand Up @@ -136,13 +131,6 @@ func (h *PublicApiHandler) AttachScalingPolicy(w http.ResponseWriter, r *http.Re
writeErrorResponse(w, http.StatusInternalServerError, "Failed to read request body")
return
}
bindingConfiguration, err := h.getBindingConfigurationFromRequest(policyBytes, logger)
if err != nil {
errMessage := "Failed to read binding configuration request body"
logger.Error(errMessage, err)
writeErrorResponse(w, http.StatusInternalServerError, errMessage)
return
}

policy, errResults := h.policyValidator.ParseAndValidatePolicy(policyBytes)
if errResults != nil {
Expand All @@ -151,6 +139,14 @@ func (h *PublicApiHandler) AttachScalingPolicy(w http.ResponseWriter, r *http.Re
return
}

bindingConfiguration, err := h.getBindingConfigurationFromRequest(policyBytes, logger)
if err != nil {
errMessage := "Failed to read binding configuration request body"
logger.Error(errMessage, err)
writeErrorResponse(w, http.StatusInternalServerError, errMessage)
return
}

policyGuid := uuid.NewString()
err = h.policydb.SaveAppPolicy(r.Context(), appId, policy, policyGuid)
if err != nil {
Expand Down Expand Up @@ -356,12 +352,8 @@ func (h *PublicApiHandler) getBindingConfigurationFromRequest(rawJson json.RawMe
if rawJson != nil {
err = json.Unmarshal(rawJson, &bindingConfiguration)
if err != nil {
actionReadBindingConfiguration := "read-binding-configurations"
logger.Error("unmarshal-binding-configuration", err)
return bindingConfiguration, apiresponses.NewFailureResponseBuilder(
ErrInvalidConfigurations, http.StatusBadRequest, actionReadBindingConfiguration).
WithErrorKey(actionReadBindingConfiguration).
Build()
return bindingConfiguration, err
}
}
return bindingConfiguration, err
Expand All @@ -371,7 +363,6 @@ func buildResponse(policy *models.ScalingPolicy, customMetricStrategy string, lo
scalingPolicyWithCustomMetricStrategy, err := models.GetBindingConfigAndPolicy(policy, customMetricStrategy)
if err != nil {
logger.Error("Failed to build policy and config response object", err)
//writeErrorResponse(w, http.StatusInternalServerError, "Error retrieving binding policy")
return nil, errors.New("error: building binding and policy")
}
responseJson, err := json.Marshal(scalingPolicyWithCustomMetricStrategy)
Expand Down
80 changes: 40 additions & 40 deletions src/autoscaler/api/publicapiserver/public_api_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,61 +382,56 @@ var _ = Describe("PublicApiHandler", func() {
Context("Binding Configuration", func() {
When("reading binding configuration from request fails", func() {
BeforeEach(func() {
pathVariables["appId"] = TEST_APP_ID
req, _ = http.NewRequest(http.MethodPut, "", bytes.NewBufferString("incorrect.json"))
req = setupRequest("incorrect.json", TEST_APP_ID, pathVariables)
})
It("should not succeed and fail with 400", func() {
Expect(resp.Code).To(Equal(http.StatusInternalServerError))
Expect(resp.Body.String()).To(ContainSubstring("Failed to read binding configuration request body"))
Expect(resp.Body.String()).To(ContainSubstring("invalid character"))
Expect(resp.Code).To(Equal(http.StatusBadRequest))
})
})

When("invalid configuration is provided with the policy", func() {
BeforeEach(func() {
pathVariables["appId"] = TEST_APP_ID
req, _ = http.NewRequest(http.MethodPut, "", bytes.NewBufferString(InvalidCustomMetricsConfigurationStr))
req = setupRequest(InvalidCustomMetricsConfigurationStr, TEST_APP_ID, pathVariables)
schedulerStatus = 200
})
It("should not succeed and fail with 400", func() {
Expect(resp.Body.String()).To(MatchJSON(`[{"context":"(root).configuration.custom_metrics.metric_submission_strategy.allow_from","description":"configuration.custom_metrics.metric_submission_strategy.allow_from must be one of the following: \"bound_app\""}]`))
Expect(resp.Code).To(Equal(http.StatusBadRequest))
Expect(resp.Body.String()).To(Equal(`[{"context":"(root).configuration.custom_metrics.metric_submission_strategy.allow_from","description":"configuration.custom_metrics.metric_submission_strategy.allow_from must be one of the following: \"bound_app\""}]`))
})
})
})

When("save configuration returned error", func() {
BeforeEach(func() {
pathVariables["appId"] = TEST_APP_ID
req, _ = http.NewRequest(http.MethodPut, "", bytes.NewBufferString(validCustomMetricsConfigurationStr))
schedulerStatus = 200
bindingdb.SetOrUpdateCustomMetricStrategyReturns(fmt.Errorf("failed to save custom metrics configuration"))
})
It("should not succeed and fail with internal server error", func() {
Expect(resp.Code).To(Equal(http.StatusInternalServerError))
Expect(resp.Body.String()).To(MatchJSON(`{"code":"Internal Server Error","message":"failed to save custom metric submission strategy in the database"}`))
})
When("save configuration returned error", func() {
BeforeEach(func() {
req = setupRequest(validCustomMetricsConfigurationStr, TEST_APP_ID, pathVariables)
schedulerStatus = 200
bindingdb.SetOrUpdateCustomMetricStrategyReturns(fmt.Errorf("failed to save custom metrics configuration"))
})
It("should not succeed and fail with internal server error", func() {
Expect(resp.Code).To(Equal(http.StatusInternalServerError))
Expect(resp.Body.String()).To(MatchJSON(`{"code":"Internal Server Error","message":"failed to save custom metric submission strategy in the database"}`))
})
})

When("valid configuration is provided with the policy", func() {
BeforeEach(func() {
pathVariables["appId"] = TEST_APP_ID
req, _ = http.NewRequest(http.MethodPut, "", bytes.NewBufferString(validCustomMetricsConfigurationStr))
schedulerStatus = 200
})
It("returns the policy and configuration with 200", func() {
Expect(resp.Code).To(Equal(http.StatusOK))
Expect(resp.Body).To(MatchJSON(validCustomMetricsConfigurationStr))
})
When("valid configuration is provided with the policy", func() {
BeforeEach(func() {
req = setupRequest(validCustomMetricsConfigurationStr, TEST_APP_ID, pathVariables)
schedulerStatus = 200
})
When("configuration is removed but only policy is provided", func() {
BeforeEach(func() {
pathVariables["appId"] = TEST_APP_ID
req, _ = http.NewRequest(http.MethodPut, "", bytes.NewBufferString(ValidPolicyStr))
schedulerStatus = 200
})
It("returns the policy 200", func() {
Expect(resp.Body).To(MatchJSON(ValidPolicyStr))
Expect(resp.Code).To(Equal(http.StatusOK))
})
It("returns the policy and configuration with 200", func() {
Expect(resp.Code).To(Equal(http.StatusOK))
Expect(resp.Body).To(MatchJSON(validCustomMetricsConfigurationStr))
})
})
When("configuration is removed but only policy is provided", func() {
BeforeEach(func() {
req = setupRequest(ValidPolicyStr, TEST_APP_ID, pathVariables)
schedulerStatus = 200
})
It("returns the policy 200", func() {
Expect(resp.Body).To(MatchJSON(ValidPolicyStr))
Expect(resp.Code).To(Equal(http.StatusOK))
})
})
})
Expand Down Expand Up @@ -1018,8 +1013,13 @@ var _ = Describe("PublicApiHandler", func() {
})
})

func setupPolicy(policydb *fakes.FakePolicyDB) {
policydb.GetAppPolicyReturns(&models.ScalingPolicy{
func setupRequest(requestBody, appId string, pathVariables map[string]string) *http.Request {
pathVariables["appId"] = appId
req, _ := http.NewRequest(http.MethodPut, "", bytes.NewBufferString(requestBody))
return req
}
func setupPolicy(policyDb *fakes.FakePolicyDB) {
policyDb.GetAppPolicyReturns(&models.ScalingPolicy{
InstanceMax: 5,
InstanceMin: 1,
ScalingRules: []*models.ScalingRule{
Expand Down

0 comments on commit 351b1ce

Please sign in to comment.