Test propagation of errors on notifications delivery

Author: Daniel Jimenez

pkg/notify/kafka_test.go

@@ -1,6 +1,7 @@
 package notify
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 	"testing"
@@ -11,6 +12,10 @@ import (
 	"github.com/adevinta/vulnerability-db/pkg/store"
 )
 
+var (
+	mockEventStreamClientErr = errors.New("mockEventStreamErr")
+)
+
 type event struct {
 	id       string
 	payload  []byte
@@ -28,10 +33,14 @@ func (e event) String() string {
 
 type mockEventStreamClient struct {
 	EventStreamClient
-	e event
+	err error
+	e   event
 }
 
 func (m *mockEventStreamClient) Push(id string, payload []byte, metadata map[string][]byte) error {
+	if m.err != nil {
+		return m.err
+	}
 	m.e = event{
 		id,
 		payload,
@@ -45,16 +54,29 @@ func (m *mockEventStreamClient) verify(want event) bool {
 }
 
 func TestKafka_PushFinding(t *testing.T) {
+	type fields struct {
+		logger         *log.Logger
+		eventStreamCli *mockEventStreamClient
+	}
+
+	logger := log.New()
+
 	testCases := []struct {
-		name string
-		f    FindingNotification
-		want event
+		name    string
+		fields  fields
+		f       FindingNotification
+		want    event
+		wantErr error
 	}{
 		{
 			// Note: Define the whole FindingNotification struct and expected JSON payload
 			// so test fails if a modification is made to the notification struct fields or
 			// JSON export definition, so we are explicitly aware of a version change.
 			name: "Should send notification with version",
+			fields: fields{
+				logger:         logger,
+				eventStreamCli: &mockEventStreamClient{},
+			},
 			f: FindingNotification{
 				FindingExpanded: store.FindingExpanded{
 					Finding: store.Finding{
@@ -192,18 +214,28 @@ func TestKafka_PushFinding(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "Should propagate eventStreamClient error",
+			fields: fields{
+				logger: logger,
+				eventStreamCli: &mockEventStreamClient{
+					err: mockEventStreamClientErr,
+				},
+			},
+			wantErr: mockEventStreamClientErr,
+		},
 	}
 
-	mock := &mockEventStreamClient{}
-	kafka := NewKafkaNotifier(mock, log.New())
-
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			if err := kafka.PushFinding(tc.f); err != nil {
-				t.Fatalf("expected no error, but got: %v", err)
+			kafka := NewKafkaNotifier(tc.fields.eventStreamCli, tc.fields.logger)
+
+			err := kafka.PushFinding(tc.f)
+			if !errors.Is(err, tc.wantErr) {
+				t.Fatalf("expected error: %v but got: %v", tc.wantErr, err)
 			}
-			if !mock.verify(tc.want) {
-				t.Fatalf("error verifying finding notification event.\ngot: %v\nwant: %v", mock.e, tc.want)
+			if !tc.fields.eventStreamCli.verify(tc.want) {
+				t.Fatalf("error verifying finding notification event.\ngot: %v\nwant: %v", tc.fields.eventStreamCli.e, tc.want)
 			}
 		})
 	}

pkg/notify/multi_test.go

@@ -41,7 +41,7 @@ func TestMulti_PushFinding(t *testing.T) {
 			wantCalled: 2,
 		},
 		{
-			name: "Should replay notification to multiple notifiers",
+			name: "Should propagate error",
 			notifiers: []*mockNotifier{
 				{},
 				{

pkg/processor/processor_test.go

@@ -24,6 +24,10 @@ const (
 	pqNotFoundErr = "sql: no rows in result set"
 )
 
+var (
+	mockNotifierErr = errors.New("mockNotifierErr")
+)
+
 type mockNotifier struct {
 	calls uint8
 }
@@ -35,9 +39,13 @@ func (n *mockNotifier) PushFinding(f notify.FindingNotification) error {
 
 type inMemMockNotifier struct {
 	sent []notify.FindingNotification
+	err  error
 }
 
 func (n *inMemMockNotifier) PushFinding(f notify.FindingNotification) error {
+	if n.err != nil {
+		return n.err
+	}
 	n.sent = append(n.sent, f)
 	return nil
 }
@@ -216,11 +224,12 @@ func TestNotifyOpenFindings(t *testing.T) {
 	}
 
 	logger := log.New()
-	tests := []struct {
+	testCases := []struct {
 		name           string
 		fields         fields
 		input          input
 		expectedNotifs []notify.FindingNotification
+		expectedErr    error
 	}{
 		{
 			name: "Should send notificatons adding check tag",
@@ -256,24 +265,45 @@ func TestNotifyOpenFindings(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "Should propagate error from notifier",
+			fields: fields{
+				logger: logger,
+				notifier: &inMemMockNotifier{
+					err: mockNotifierErr,
+				},
+				store: &mockStore{
+					returnFindingsExpanded: []store.FindingExpanded{
+						{Finding: store.Finding{ID: "1"}},
+					},
+				},
+			},
+			input: input{
+				findingsState: []store.FindingState{
+					{ID: "1"},
+				},
+				tag: "test-tag",
+			},
+			expectedErr: mockNotifierErr,
+		},
 	}
 
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
 			processor := &CheckProcessor{
-				notifier:      tt.fields.notifier,
-				store:         tt.fields.store,
-				resultsClient: tt.fields.resClient,
-				logger:        tt.fields.logger,
+				notifier:      tc.fields.notifier,
+				store:         tc.fields.store,
+				resultsClient: tc.fields.resClient,
+				logger:        tc.fields.logger,
 			}
 
-			err := processor.notifyFindings(tt.input.findingsState, tt.input.tag)
-			if err != nil {
-				t.Fatalf("expected no error but got: %v", err)
+			err := processor.notifyFindings(tc.input.findingsState, tc.input.tag)
+			if !errors.Is(err, tc.expectedErr) {
+				t.Fatalf("expected error: %v but got: %v", tc.expectedErr, err)
 			}
-			got := tt.fields.notifier.(*inMemMockNotifier).sent
-			if !reflect.DeepEqual(tt.expectedNotifs, got) {
-				t.Errorf("expected:\n%v\nbut got:\n%v", got, tt.expectedNotifs)
+			got := tc.fields.notifier.(*inMemMockNotifier).sent
+			if !reflect.DeepEqual(tc.expectedNotifs, got) {
+				t.Fatalf("expected:\n%v\nbut got:\n%v", got, tc.expectedNotifs)
 			}
 		})
 	}