isolate fix to mercury; more tests

.changeset/big-camels-report.md

@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+#bugfix fix missing unregister in mercury job loop

core/services/ocr2/plugins/mercury/plugin.go

@@ -80,14 +80,13 @@ func NewServices(
 		return nil, errors.New("expected job to have a non-nil PipelineSpec")
 	}
 
-	var err error
 	var pluginConfig config.PluginConfig
 	if len(jb.OCR2OracleSpec.PluginConfig) == 0 {
 		if !enableTriggerCapability {
 			return nil, fmt.Errorf("at least one transmission option must be configured")
 		}
 	} else {
-		err = json.Unmarshal(jb.OCR2OracleSpec.PluginConfig.Bytes(), &pluginConfig)
+		err := json.Unmarshal(jb.OCR2OracleSpec.PluginConfig.Bytes(), &pluginConfig)
 		if err != nil {
 			return nil, errors.WithStack(err)
 		}
@@ -102,8 +101,8 @@ func NewServices(
 	// encapsulate all the subservices and ensure we close them all if any fail to start
 	srvs := []job.ServiceCtx{ocr2Provider}
 	abort := func() {
-		if err = services.MultiCloser(srvs).Close(); err != nil {
-			lggr.Errorw("Error closing unused services", "err", err)
+		if cerr := services.MultiCloser(srvs).Close(); cerr != nil {
+			lggr.Errorw("Error closing unused services", "err", cerr)
 		}
 	}
 	saver := ocrcommon.NewResultRunSaver(pipelineRunner, lggr, cfg.MaxSuccessfulRuns(), cfg.ResultWriteQueueDepth())
@@ -113,6 +112,7 @@ func NewServices(
 	var (
 		factory         ocr3types.MercuryPluginFactory
 		factoryServices []job.ServiceCtx
+		fErr            error
 	)
 	fCfg := factoryCfg{
 		orm:                   orm,
@@ -128,31 +128,31 @@ func NewServices(
 	}
 	switch feedID.Version() {
 	case 1:
-		factory, factoryServices, err = newv1factory(fCfg)
-		if err != nil {
+		factory, factoryServices, fErr = newv1factory(fCfg)
+		if fErr != nil {
 			abort()
-			return nil, fmt.Errorf("failed to create mercury v1 factory: %w", err)
+			return nil, fmt.Errorf("failed to create mercury v1 factory: %w", fErr)
 		}
 		srvs = append(srvs, factoryServices...)
 	case 2:
-		factory, factoryServices, err = newv2factory(fCfg)
-		if err != nil {
+		factory, factoryServices, fErr = newv2factory(fCfg)
+		if fErr != nil {
 			abort()
-			return nil, fmt.Errorf("failed to create mercury v2 factory: %w", err)
+			return nil, fmt.Errorf("failed to create mercury v2 factory: %w", fErr)
 		}
 		srvs = append(srvs, factoryServices...)
 	case 3:
-		factory, factoryServices, err = newv3factory(fCfg)
-		if err != nil {
+		factory, factoryServices, fErr = newv3factory(fCfg)
+		if fErr != nil {
 			abort()
-			return nil, fmt.Errorf("failed to create mercury v3 factory: %w", err)
+			return nil, fmt.Errorf("failed to create mercury v3 factory: %w", fErr)
 		}
 		srvs = append(srvs, factoryServices...)
 	case 4:
-		factory, factoryServices, err = newv4factory(fCfg)
-		if err != nil {
+		factory, factoryServices, fErr = newv4factory(fCfg)
+		if fErr != nil {
 			abort()
-			return nil, fmt.Errorf("failed to create mercury v4 factory: %w", err)
+			return nil, fmt.Errorf("failed to create mercury v4 factory: %w", fErr)
 		}
 		srvs = append(srvs, factoryServices...)
 	default:

core/services/ocr2/plugins/mercury/plugin_test.go

@@ -2,6 +2,7 @@ package mercury_test
 
 import (
 	"context"
+	"errors"
 	"os/exec"
 	"reflect"
 	"testing"
@@ -101,6 +102,7 @@ func TestNewServices(t *testing.T) {
 	type args struct {
 		pluginConfig job.JSONConfig
 		feedID       utils.FeedID
+		cfg          mercuryocr2.Config
 	}
 	testCases := []struct {
 		name            string
@@ -109,6 +111,7 @@ func TestNewServices(t *testing.T) {
 		wantLoopFactory any
 		wantServiceCnt  int
 		wantErr         bool
+		wantErrStr      string
 	}{
 		{
 			name: "no plugin config error ",
@@ -188,6 +191,19 @@ func TestNewServices(t *testing.T) {
 			wantErr:         false,
 			wantLoopFactory: &loop.MercuryV3Service{},
 		},
+		{
+			name:     "v3 loop err",
+			loopMode: true,
+			args: args{
+				pluginConfig: v3jsonCfg,
+				feedID:       v3FeedId,
+				cfg:          mercuryocr2.NewMercuryConfig(1, 1, &testRegistrarConfig{failRegister: true}),
+			},
+			wantServiceCnt:  expectedLoopServiceCnt,
+			wantErr:         true,
+			wantLoopFactory: &loop.MercuryV3Service{},
+			wantErrStr:      "failed to init loop for feed",
+		},
 		{
 			name:     "v4 loop",
 			loopMode: true,
@@ -206,11 +222,21 @@ func TestNewServices(t *testing.T) {
 				t.Setenv(string(env.MercuryPlugin.Cmd), "fake_cmd")
 				assert.NotEmpty(t, env.MercuryPlugin.Cmd.Get())
 			}
-			got, err := newServicesTestWrapper(t, tt.args.pluginConfig, tt.args.feedID)
+			// use default config if not provided
+			if tt.args.cfg == nil {
+				tt.args.cfg = testCfg
+			}
+			got, err := newServicesTestWrapper(t, tt.args.pluginConfig, tt.args.feedID, tt.args.cfg)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("NewServices() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
+			if err != nil {
+				if tt.wantErrStr != "" {
+					assert.Contains(t, err.Error(), tt.wantErrStr)
+				}
+				return
+			}
 			assert.Len(t, got, tt.wantServiceCnt)
 			if tt.loopMode {
 				foundLoopFactory := false
@@ -310,11 +336,11 @@ func TestNewServices(t *testing.T) {
 
 // we are only varying the version via feedID (and the plugin config)
 // this wrapper supplies dummy values for the rest of the arguments
-func newServicesTestWrapper(t *testing.T, pluginConfig job.JSONConfig, feedID utils.FeedID) ([]job.ServiceCtx, error) {
+func newServicesTestWrapper(t *testing.T, pluginConfig job.JSONConfig, feedID utils.FeedID, cfg mercuryocr2.Config) ([]job.ServiceCtx, error) {
 	t.Helper()
 	jb := testJob
 	jb.OCR2OracleSpec.PluginConfig = pluginConfig
-	return mercuryocr2.NewServices(jb, &testProvider{}, nil, logger.TestLogger(t), testArgsNoPlugin, testCfg, nil, &testDataSourceORM{}, feedID, false)
+	return mercuryocr2.NewServices(jb, &testProvider{}, nil, logger.TestLogger(t), testArgsNoPlugin, cfg, nil, &testDataSourceORM{}, feedID, false)
 }
 
 type testProvider struct{}
@@ -380,12 +406,17 @@ func (*testProvider) Start(context.Context) error { return nil }
 
 var _ commontypes.MercuryProvider = (*testProvider)(nil)
 
-type testRegistrarConfig struct{}
+type testRegistrarConfig struct {
+	failRegister bool
+}
 
 func (c *testRegistrarConfig) UnregisterLOOP(ID string) {}
 
 // RegisterLOOP implements plugins.RegistrarConfig.
-func (*testRegistrarConfig) RegisterLOOP(config plugins.CmdConfig) (func() *exec.Cmd, loop.GRPCOpts, error) {
+func (c *testRegistrarConfig) RegisterLOOP(config plugins.CmdConfig) (func() *exec.Cmd, loop.GRPCOpts, error) {
+	if c.failRegister {
+		return nil, loop.GRPCOpts{}, errors.New("failed to register")
+	}
 	return nil, loop.GRPCOpts{}, nil
 }