Improve error messages for Assertions (#1726)

* Improve error messages for Assertions

Specifically for Assertions which define "Message". Now the test failure
properly notifies the user that the Assertion was looking for violations
for a particular message.

Also add several unit tests for Assertions to cover these cases. Unit
test coverage is now 100% for assertions.go. We could probably add more
tests for assertions, but I'd rather that be its own PR. What I've added
is for the behavior changed by this PR.

Also add validation that "violations" isn't set to a negative number.

Fixes #1639

Signed-off-by: Will Beason <willbeason@google.com>

* Fix linter errors

Signed-off-by: Will Beason <willbeason@google.com>

pkg/gktest/assertion.go

@@ -67,6 +67,10 @@ func (a *Assertion) Run(results []*types.Result) error {
 func (a *Assertion) matchesCount(matching int32) error {
 	switch a.Violations.Type {
 	case intstr.Int:
+		if a.Violations.IntVal < 0 {
+			return fmt.Errorf(`%w: assertion.violation, if set, must be a nonnegative integer, "yes", or "no"`,
+				ErrInvalidYAML)
+		}
 		return a.matchesCountInt(matching)
 	case intstr.String:
 		return a.matchesCountStr(matching)
@@ -81,6 +85,10 @@ func (a *Assertion) matchesCount(matching int32) error {
 func (a *Assertion) matchesCountInt(matching int32) error {
 	wantMatching := a.Violations.IntVal
 	if wantMatching != matching {
+		if a.Message != nil {
+			return fmt.Errorf("%w: got %d violations containing %q but want exactly %d",
+				ErrNumViolations, matching, *a.Message, wantMatching)
+		}
 		return fmt.Errorf("%w: got %d violations but want exactly %d",
 			ErrNumViolations, matching, wantMatching)
 	}
@@ -92,20 +100,28 @@ func (a *Assertion) matchesCountStr(matching int32) error {
 	switch a.Violations.StrVal {
 	case "yes":
 		if matching == 0 {
+			if a.Message != nil {
+				return fmt.Errorf("%w: got %d violations containing %q but want at least %d",
+					ErrNumViolations, matching, *a.Message, 1)
+			}
 			return fmt.Errorf("%w: got %d violations but want at least %d",
 				ErrNumViolations, matching, 1)
 		}
 
 		return nil
 	case "no":
 		if matching > 0 {
+			if a.Message != nil {
+				return fmt.Errorf("%w: got %d violations containing %q but want none",
+					ErrNumViolations, matching, *a.Message)
+			}
 			return fmt.Errorf("%w: got %d violations but want none",
 				ErrNumViolations, matching)
 		}
 
 		return nil
 	default:
-		return fmt.Errorf(`%w: assertion.violation, if set, must be an integer, "yes", or "no"`,
+		return fmt.Errorf(`%w: assertion.violation, if set, must be a nonnegative integer, "yes", or "no"`,
 			ErrInvalidYAML)
 	}
 }

pkg/gktest/assertion_test.go

@@ -0,0 +1,73 @@
+package gktest
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/open-policy-agent/frameworks/constraint/pkg/types"
+	"k8s.io/utils/pointer"
+)
+
+func TestAssertion_Run(t *testing.T) {
+	tests := []struct {
+		name      string
+		assertion *Assertion
+		results   []*types.Result
+		wantErr   error
+	}{{
+		name:      "default to expect violation",
+		assertion: &Assertion{},
+		results:   nil,
+		wantErr:   ErrNumViolations,
+	}, {
+		name: "no violations",
+		assertion: &Assertion{
+			Violations: intStrFromInt(0),
+		},
+		results: nil,
+		wantErr: nil,
+	}, {
+		name: "negative violations",
+		assertion: &Assertion{
+			Violations: intStrFromInt(-1),
+		},
+		results: nil,
+		wantErr: ErrInvalidYAML,
+	}, {
+		name: "violation with message",
+		assertion: &Assertion{
+			Violations: intStrFromInt(1),
+			Message:    pointer.StringPtr("message"),
+		},
+		results: nil,
+		wantErr: ErrNumViolations,
+	}, {
+		name: "no violations with message",
+		assertion: &Assertion{
+			Violations: intStrFromStr("no"),
+			Message:    pointer.StringPtr("message"),
+		},
+		results: nil,
+		wantErr: nil,
+	}, {
+		name: "fail no violations with message",
+		assertion: &Assertion{
+			Violations: intStrFromStr("no"),
+			Message:    pointer.StringPtr("message"),
+		},
+		results: []*types.Result{{
+			Msg: "message",
+		}},
+		wantErr: ErrNumViolations,
+	}}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := tt.assertion.Run(tt.results)
+
+			if !errors.Is(err, tt.wantErr) {
+				t.Errorf("Run() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}