Deduplicate event handler and executor unit tests (#3740)

This follows on from the previous PR: simplify the event handler tests
to the minimum. This means that the event handling tests (which have
asynchronous behaviour and may time out) will not fail when changes are
made to the executor logic.

internal/engine/eval_status.go

@@ -32,7 +32,7 @@ import (
 	pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
 )
 
-func (e *Executor) createEvalStatusParams(
+func (e *executor) createEvalStatusParams(
 	ctx context.Context,
 	inf *entities.EntityInfoWrapper,
 	profile *pb.Profile,
@@ -111,7 +111,7 @@ func (e *Executor) createEvalStatusParams(
 //
 // If the error in the evaluation status resolves to an errors.ErrEvaluationSkipSilently,
 // no details are stored or logged.
-func (e *Executor) createOrUpdateEvalStatus(
+func (e *executor) createOrUpdateEvalStatus(
 	ctx context.Context,
 	params *engif.EvalStatusParams,
 ) error {

internal/engine/executor.go

@@ -39,8 +39,14 @@ import (
 	provinfv1 "github.com/stacklok/minder/pkg/providers/v1"
 )
 
+//go:generate go run go.uber.org/mock/mockgen -package mock_$GOPACKAGE -destination=./mock/$GOFILE -source=./$GOFILE
+
 // Executor is the engine that executes the rules for a given event
-type Executor struct {
+type Executor interface {
+	EvalEntityEvent(ctx context.Context, inf *entities.EntityInfoWrapper) error
+}
+
+type executor struct {
 	querier         db.Store
 	providerManager manager.ProviderManager
 	metrics         *ExecutorMetrics
@@ -55,8 +61,8 @@ func NewExecutor(
 	metrics *ExecutorMetrics,
 	historyService history.EvaluationHistoryService,
 	featureFlags openfeature.IClient,
-) *Executor {
-	return &Executor{
+) Executor {
+	return &executor{
 		querier:         querier,
 		providerManager: providerManager,
 		metrics:         metrics,
@@ -67,7 +73,7 @@ func NewExecutor(
 
 // EvalEntityEvent evaluates the entity specified in the EntityInfoWrapper
 // against all relevant rules in the project hierarchy.
-func (e *Executor) EvalEntityEvent(ctx context.Context, inf *entities.EntityInfoWrapper) error {
+func (e *executor) EvalEntityEvent(ctx context.Context, inf *entities.EntityInfoWrapper) error {
 	logger := zerolog.Ctx(ctx).Info().
 		Str("entity_type", inf.Type.ToString()).
 		Str("execution_id", inf.ExecutionID.String()).
@@ -148,7 +154,7 @@ func (e *Executor) EvalEntityEvent(ctx context.Context, inf *entities.EntityInfo
 	return nil
 }
 
-func (e *Executor) forProjectsInHierarchy(
+func (e *executor) forProjectsInHierarchy(
 	ctx context.Context,
 	inf *entities.EntityInfoWrapper,
 	f func(context.Context, *pb.Profile, []uuid.UUID) error,
@@ -176,7 +182,7 @@ func (e *Executor) forProjectsInHierarchy(
 	return nil
 }
 
-func (e *Executor) getEvaluator(
+func (e *executor) getEvaluator(
 	ctx context.Context,
 	inf *entities.EntityInfoWrapper,
 	provider provinfv1.Provider,
@@ -234,7 +240,7 @@ func (e *Executor) getEvaluator(
 	return params, rte, actionEngine, nil
 }
 
-func (e *Executor) updateLockLease(
+func (e *executor) updateLockLease(
 	ctx context.Context,
 	executionID uuid.UUID,
 	params *engif.EvalStatusParams,
@@ -256,7 +262,7 @@ func (e *Executor) updateLockLease(
 	logger.Info().Msg("lock lease updated")
 }
 
-func (e *Executor) releaseLockAndFlush(
+func (e *executor) releaseLockAndFlush(
 	ctx context.Context,
 	inf *entities.EntityInfoWrapper,
 ) {

internal/engine/executor_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/sqlc-dev/pqtype"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/mock/gomock"
+	"golang.org/x/oauth2"
 	"google.golang.org/protobuf/types/known/structpb"
 
 	mockdb "github.com/stacklok/minder/database/mock"
@@ -36,9 +37,12 @@ import (
 	"github.com/stacklok/minder/internal/crypto"
 	"github.com/stacklok/minder/internal/db"
 	"github.com/stacklok/minder/internal/engine"
+	"github.com/stacklok/minder/internal/engine/actions/alert"
+	"github.com/stacklok/minder/internal/engine/actions/remediate"
 	"github.com/stacklok/minder/internal/engine/entities"
 	"github.com/stacklok/minder/internal/flags"
 	mock_history "github.com/stacklok/minder/internal/history/mock"
+	"github.com/stacklok/minder/internal/logger"
 	"github.com/stacklok/minder/internal/metrics/meters"
 	"github.com/stacklok/minder/internal/providers"
 	"github.com/stacklok/minder/internal/providers/github/clients"
@@ -330,10 +334,43 @@ default allow = true`,
 		}).WithRepositoryID(repositoryID).
 		WithExecutionID(executionID)
 
-	err = executor.EvalEntityEvent(context.Background(), eiw)
+	ts := &logger.TelemetryStore{
+		Project:    projectID,
+		ProviderID: providerID,
+		Repository: repositoryID,
+	}
+	ctx := ts.WithTelemetry(context.Background())
+
+	err = executor.EvalEntityEvent(ctx, eiw)
 	require.NoError(t, err)
 
-	// Note: Assertions currently rely on the DB expectations
-	// TODO: Consider refactoring the code further to make it easier to
-	// test.
+	require.Len(t, ts.Evals, 1, "expected one eval to be logged")
+	requredEval := ts.Evals[0]
+	require.Equal(t, "test-profile", requredEval.Profile.Name)
+	require.Equal(t, "success", requredEval.EvalResult)
+	require.Equal(t, "passthrough", requredEval.RuleType.Name)
+	require.Equal(t, "off", requredEval.Actions[alert.ActionType].State)
+	require.Equal(t, "off", requredEval.Actions[remediate.ActionType].State)
+}
+
+func generateFakeAccessToken(t *testing.T, cryptoEngine crypto.Engine) pqtype.NullRawMessage {
+	t.Helper()
+
+	ftoken := &oauth2.Token{
+		AccessToken:  "foo-bar",
+		TokenType:    "bar-baz",
+		RefreshToken: "",
+		// Expires in 10 mins
+		Expiry: time.Now().Add(10 * time.Minute),
+	}
+
+	// encrypt token
+	encryptedToken, err := cryptoEngine.EncryptOAuthToken(ftoken)
+	require.NoError(t, err)
+	serialized, err := encryptedToken.Serialize()
+	require.NoError(t, err)
+	return pqtype.NullRawMessage{
+		RawMessage: serialized,
+		Valid:      true,
+	}
 }

internal/engine/handler.go

@@ -47,15 +47,15 @@ type ExecutorEventHandler struct {
 	// terminationcontext is used to terminate the executor
 	// when the server is shutting down.
 	terminationcontext context.Context
-	executor           *Executor
+	executor           Executor
 }
 
-// NewExecutorEventHandler creates the event handler for the Executor
+// NewExecutorEventHandler creates the event handler for the executor
 func NewExecutorEventHandler(
 	ctx context.Context,
 	evt events.Publisher,
 	handlerMiddleware []message.HandlerMiddleware,
-	executor *Executor,
+	executor Executor,
 ) *ExecutorEventHandler {
 	return &ExecutorEventHandler{
 		evt:                    evt,