Repository: apigee/registry
Pull request #689 (2022-08-18 22:52:31)

Fix out of range error for scores
Fixes https://github.com/apigee/registry/issues/688

codecov[bot]: # [Codecov](https://codecov.io/gh/apigee/registry/pull/689?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) Report
> Merging [#689](https://codecov.io/gh/apigee/registry/pull/689?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) (b6b1af9) into [main](https://codecov.io/gh/apigee/registry/commit/8f8da52a259197d2053e347b4be84a3441f2b69c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) (8f8da52) will **increase** coverage by `0.12%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main     #689      +/-   ##
==========================================
+ Coverage   55.26%   55.38%   +0.12%     
==========================================
  Files          93       93              
  Lines        7917     7913       -4     
==========================================
+ Hits         4375     4383       +8     
+ Misses       3070     3062       -8     
+ Partials      472      468       -4     
```


| [Impacted Files](https://codecov.io/gh/apigee/registry/pull/689?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) | Coverage Δ | |
|---|---|---|
| [cmd/registry/scoring/score.go](https://codecov.io/gh/apigee/registry/pull/689/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L3Njb3Jpbmcvc2NvcmUuZ28=) | `74.31% <100.00%> (+3.31%)` | :arrow_up: |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee)


Patch:
diff --git a/cmd/registry/scoring/score.go b/cmd/registry/scoring/score.go
index 28d712a..a235fa8 100644
--- a/cmd/registry/scoring/score.go
+++ b/cmd/registry/scoring/score.go
@@ -339,15 +339,8 @@ func processScoreType(definition *rpc.ScoreDefinition, scoreValue interface{}, p
 			return nil, fmt.Errorf("failed typecheck for output: expected either int64 or float64 got %s (type: %T)", v, v)
 		}
 
-		// Check that the scoreValue is within min/max limits
 		configuredMin := definition.GetInteger().GetMinValue() // 0 if not set
 		configuredMax := definition.GetInteger().GetMaxValue() // 0 if not set
-		if value < configuredMin {
-			return nil, fmt.Errorf("evaluated score value(%d) cannot be less than the configured min_value (%d)", value, configuredMin)
-		}
-		if value > configuredMax {
-			return nil, fmt.Errorf("evaluated score value(%d) cannot be greater than the configured max_value (%d)", value, configuredMax)
-		}
 
 		// Populate Value field in Score proto
 		score.Value = &rpc.Score_IntegerValue{
@@ -358,6 +351,12 @@ func processScoreType(definition *rpc.ScoreDefinition, scoreValue interface{}, p
 			},
 		}
 
+		// Check that the scoreValue is within min/max limits and assign default ALERT Severity
+		if value < configuredMin || value > configuredMax {
+			score.Severity = rpc.Severity_ALERT
+			break
+		}
+
 		// Populate the severity field according to Thresholds
 		for _, t := range definition.GetInteger().GetThresholds() {
 			if value >= t.GetRange().GetMin() && value <= t.GetRange().GetMax() {
@@ -382,14 +381,6 @@ func processScoreType(definition *rpc.ScoreDefinition, scoreValue interface{}, p
 			return nil, fmt.Errorf("failed typecheck for output: expected either int64 or float64 got %s (type: %T)", v, v)
 		}
 
-		// Check that the scoreValue is within min/max limits
-		if value < 0 {
-			return nil, fmt.Errorf("evaluated score value(%f) cannot be less than 0", value)
-		}
-		if value > 100 {
-			return nil, fmt.Errorf("evaluated score value(%f) cannot be greater than 100", value)
-		}
-
 		// Populate Value field in Score proto
 		score.Value = &rpc.Score_PercentValue{
 			PercentValue: &rpc.PercentValue{
@@ -397,6 +388,12 @@ func processScoreType(definition *rpc.ScoreDefinition, scoreValue interface{}, p
 			},
 		}
 
+		// Check that the scoreValue is within min/max limits and assign default ALERT Severity
+		if value < 0 || value > 100 {
+			score.Severity = rpc.Severity_ALERT
+			break
+		}
+
 		// Populate the severity field according to Thresholds
 		for _, t := range definition.GetPercent().GetThresholds() {
 			if value >= float32(t.GetRange().GetMin()) && value <= float32(t.GetRange().GetMax()) {


Test patch:
diff --git a/cmd/registry/scoring/score_test.go b/cmd/registry/scoring/score_test.go
index b343ddb..6606e7b 100644
--- a/cmd/registry/scoring/score_test.go
+++ b/cmd/registry/scoring/score_test.go
@@ -1203,6 +1203,50 @@ func TestProcessScoreType(t *testing.T) {
 				},
 			},
 		},
+		{
+			desc:       "greater than max integer",
+			definition: integerDefinition,
+			scoreValue: int64(11),
+			wantScore: &rpc.Score{
+				Id:             "score-lint-error",
+				Kind:           "Score",
+				DisplayName:    "Lint Error",
+				Description:    "Number of errors found by linter",
+				Uri:            "http://some/test/uri",
+				UriDisplayName: "Test URI",
+				DefinitionName: "projects/score-type-test/locations/global/artifacts/lint-error",
+				Severity:       rpc.Severity_ALERT,
+				Value: &rpc.Score_IntegerValue{
+					IntegerValue: &rpc.IntegerValue{
+						Value:    11,
+						MinValue: 0,
+						MaxValue: 10,
+					},
+				},
+			},
+		},
+		{
+			desc:       "less than min integer",
+			definition: integerDefinition,
+			scoreValue: int64(-1),
+			wantScore: &rpc.Score{
+				Id:             "score-lint-error",
+				Kind:           "Score",
+				DisplayName:    "Lint Error",
+				Description:    "Number of errors found by linter",
+				Uri:            "http://some/test/uri",
+				UriDisplayName: "Test URI",
+				DefinitionName: "projects/score-type-test/locations/global/artifacts/lint-error",
+				Severity:       rpc.Severity_ALERT,
+				Value: &rpc.Score_IntegerValue{
+					IntegerValue: &rpc.IntegerValue{
+						Value:    -1,
+						MinValue: 0,
+						MaxValue: 10,
+					},
+				},
+			},
+		},
 		{
 			desc:       "happy path percent",
 			definition: percentDefinition,
@@ -1243,6 +1287,46 @@ func TestProcessScoreType(t *testing.T) {
 				},
 			},
 		},
+		{
+			desc:       "greater than max percent",
+			definition: percentDefinition,
+			scoreValue: int64(101),
+			wantScore: &rpc.Score{
+				Id:             "score-lint-error-percent",
+				Kind:           "Score",
+				DisplayName:    "Lint Error Percentage",
+				Description:    "Percentage errors found by linter",
+				Uri:            "http://some/test/uri",
+				UriDisplayName: "Test URI",
+				DefinitionName: "projects/score-type-test/locations/global/artifacts/lint-error-percent",
+				Severity:       rpc.Severity_ALERT,
+				Value: &rpc.Score_PercentValue{
+					PercentValue: &rpc.PercentValue{
+						Value: 101,
+					},
+				},
+			},
+		},
+		{
+			desc:       "less than min percent",
+			definition: percentDefinition,
+			scoreValue: int64(-1),
+			wantScore: &rpc.Score{
+				Id:             "score-lint-error-percent",
+				Kind:           "Score",
+				DisplayName:    "Lint Error Percentage",
+				Description:    "Percentage errors found by linter",
+				Uri:            "http://some/test/uri",
+				UriDisplayName: "Test URI",
+				DefinitionName: "projects/score-type-test/locations/global/artifacts/lint-error-percent",
+				Severity:       rpc.Severity_ALERT,
+				Value: &rpc.Score_PercentValue{
+					PercentValue: &rpc.PercentValue{
+						Value: -1,
+					},
+				},
+			},
+		},
 		{
 			desc:       "happy path boolean",
 			definition: booleanDefinition,
@@ -1291,35 +1375,15 @@ func TestProcessScoreTypeError(t *testing.T) {
 			definition: integerDefinition,
 			scoreValue: true,
 		},
-		{
-			desc:       "greater than max integer",
-			definition: integerDefinition,
-			scoreValue: 11,
-		},
-		{
-			desc:       "less than min integer",
-			definition: integerDefinition,
-			scoreValue: -1,
-		},
 		{
 			desc:       "type mismatch percent",
 			definition: percentDefinition,
 			scoreValue: false,
 		},
-		{
-			desc:       "greater than max percent",
-			definition: integerDefinition,
-			scoreValue: 101,
-		},
-		{
-			desc:       "less than min percent",
-			definition: integerDefinition,
-			scoreValue: -1,
-		},
 		{
 			desc:       "type mismatch boolean",
 			definition: booleanDefinition,
-			scoreValue: 1,
+			scoreValue: int64(1),
 		},
 	}
 	for _, test := range tests {