From 01cd46eead4a84510503e0d977069535d01b68d1 Mon Sep 17 00:00:00 2001 From: Owen Cabalceta Date: Mon, 11 Nov 2024 13:51:07 -0500 Subject: [PATCH] patch: remove forloop var pass by reference Sucessful `testVerifyEnviromentRegistrars` with patch: ``` === RUN TestNewEnvironment === RUN TestNewEnvironment/Verify_Multi_Registrar_Environment --- PASS: TestNewEnvironment/Verify_Multi_Registrar_Environment (0.00s) --- PASS: TestNewEnvironment (0.00s) PASS ok github.com/xmidt-org/webpa-common/v2/service/consul 0.019s ``` Failure `testVerifyEnviromentRegistrars` with forloop bug: ``` panic: assert: mock: The method has been called over 1 times. Either do one more Mock.On("Register").Return(...), or remove extra call. This call was unexpected: Register(*api.AgentServiceRegistration) 0: &api.AgentServiceRegistration{Kind:"", ID:"api:deadbeef", Name:"deadbeef-api", Tags:[]string{"role=deadbeef, region=1"}, Port:443, Address:"deadbeef.com", SocketPath:"", TaggedAddresses:map[string]api.ServiceAddress(nil), EnableTagOverride:false, Meta:map[string]string(nil), Weights:(*api.AgentWeights)(nil), Check:(*api.AgentServiceCheck)(nil), Checks:api.AgentServiceChecks(nil), Proxy:(*api.AgentServiceConnectProxyConfig)(nil), Connect:(*api.AgentServiceConnect)(nil), Namespace:"", Partition:"", Locality:(*api.Locality)(nil)} at: [/Users/odc/Documents/GitHub/xmidt-org/webpa-common/service/consul/mocks_test.go:60 /Users/odc/go/pkg/mod/github.com/go-kit/kit@v0.13.0/sd/consul/registrar.go:30 /Users/odc/Documents/GitHub/xmidt-org/webpa-common/service/registrars.go:13 /Users/odc/Documents/GitHub/xmidt-org/webpa-common/service/environment.go:198 /Users/odc/Documents/GitHub/xmidt-org/webpa-common/service/consul/environment_test.go:191] [recovered] panic: assert: mock: The method has been called over 1 times. Either do one more Mock.On("Register").Return(...), or remove extra call. This call was unexpected: Register(*api.AgentServiceRegistration) 0: &api.AgentServiceRegistration{Kind:"", ID:"api:deadbeef", Name:"deadbeef-api", Tags:[]string{"role=deadbeef, region=1"}, Port:443, Address:"deadbeef.com", SocketPath:"", TaggedAddresses:map[string]api.ServiceAddress(nil), EnableTagOverride:false, Meta:map[string]string(nil), Weights:(*api.AgentWeights)(nil), Check:(*api.AgentServiceCheck)(nil), Checks:api.AgentServiceChecks(nil), Proxy:(*api.AgentServiceConnectProxyConfig)(nil), Connect:(*api.AgentServiceConnect)(nil), Namespace:"", Partition:"", Locality:(*api.Locality)(nil)} at: [/Users/odc/Documents/GitHub/xmidt-org/webpa-common/service/consul/mocks_test.go:60 /Users/odc/go/pkg/mod/github.com/go-kit/kit@v0.13.0/sd/consul/registrar.go:30 /Users/odc/Documents/GitHub/xmidt-org/webpa-common/service/registrars.go:13 /Users/odc/Documents/GitHub/xmidt-org/webpa-common/service/environment.go:198 /Users/odc/Documents/GitHub/xmidt-org/webpa-common/service/consul/environment_test.go:191] ``` --- service/consul/environment.go | 2 +- service/consul/environment_test.go | 60 ++++++++++++++++++++++++++++++ service/consul/registrar.go | 4 +- service/consul/registrar_test.go | 14 +++---- 4 files changed, 70 insertions(+), 10 deletions(-) diff --git a/service/consul/environment.go b/service/consul/environment.go index 00e8b2cb..2f6e2e88 100644 --- a/service/consul/environment.go +++ b/service/consul/environment.go @@ -167,7 +167,7 @@ func newRegistrars(l *adapter.Logger, registrationScheme string, c gokitconsul.C rid := zap.String("id", registration.ID) in := zap.String("instance", instance) l.Logger = l.Logger.With(rid, in) - consulRegistrar, err = NewRegistrar(c, u, ®istration, l) + consulRegistrar, err = NewRegistrar(c, u, registration, l) if err != nil { return } diff --git a/service/consul/environment_test.go b/service/consul/environment_test.go index 490de2af..126d3aab 100644 --- a/service/consul/environment_test.go +++ b/service/consul/environment_test.go @@ -1,6 +1,7 @@ package consul import ( + "reflect" "testing" "github.com/hashicorp/consul/api" @@ -8,6 +9,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/xmidt-org/webpa-common/v2/adapter" + "github.com/xmidt-org/webpa-common/v2/service" ) @@ -136,8 +138,66 @@ func testNewEnvironmentFull(t *testing.T) { ttlUpdater.AssertExpectations(t) } +func testVerifyEnviromentRegistrars(t *testing.T) { + var ( + require = require.New(t) + logger = adapter.DefaultLogger() + clientFactory = prepareMockClientFactory() + client = new(mockClient) + ttlUpdater = new(mockTTLUpdater) + registrations = []api.AgentServiceRegistration{ + { + ID: "deadbeef", + Name: "deadbeef", + Tags: []string{"role=deadbeef, region=1"}, + Address: "deadbeef.com", + Port: 8080, + }, + { + ID: "api:deadbeef", + Name: "deadbeef-api", + Tags: []string{"role=deadbeef, region=1"}, + Address: "deadbeef.com", + Port: 443, + }, + } + co = Options{ + Client: &api.Config{ + Address: "localhost:8500", + Scheme: "https", + }, + Registrations: registrations, + } + ) + + clientFactory.On("NewClient", mock.MatchedBy(func(*api.Client) bool { return true })).Return(client, ttlUpdater).Once() + + client.On("Register", mock.MatchedBy(func(r *api.AgentServiceRegistration) bool { + return reflect.DeepEqual(registrations[0], *r) + })).Return(error(nil)).Once() + client.On("Register", mock.MatchedBy(func(r *api.AgentServiceRegistration) bool { + return reflect.DeepEqual(registrations[1], *r) + })).Return(error(nil)).Once() + + client.On("Deregister", mock.MatchedBy(func(r *api.AgentServiceRegistration) bool { + return reflect.DeepEqual(registrations[0], *r) + })).Return(error(nil)).Once() + client.On("Deregister", mock.MatchedBy(func(r *api.AgentServiceRegistration) bool { + return reflect.DeepEqual(registrations[1], *r) + })).Return(error(nil)).Once() + consulEnv, err := NewEnvironment(logger, service.DefaultScheme, co) + require.NoError(err) + + consulEnv.Register() + consulEnv.Deregister() + clientFactory.AssertExpectations(t) + ttlUpdater.AssertExpectations(t) + client.AssertExpectations(t) +} + func TestNewEnvironment(t *testing.T) { t.Run("Empty", testNewEnvironmentEmpty) t.Run("ClientError", testNewEnvironmentClientError) t.Run("Full", testNewEnvironmentFull) + t.Run("Verify Multi Registrar Environment", testVerifyEnviromentRegistrars) } diff --git a/service/consul/registrar.go b/service/consul/registrar.go index cad90b18..ad6742ba 100644 --- a/service/consul/registrar.go +++ b/service/consul/registrar.go @@ -130,7 +130,7 @@ type ttlRegistrar struct { } // NewRegistrar creates an sd.Registrar, binding any TTL checks to the Register/Deregister lifecycle as needed. -func NewRegistrar(c gokitconsul.Client, u ttlUpdater, r *api.AgentServiceRegistration, logger *adapter.Logger) (sd.Registrar, error) { +func NewRegistrar(c gokitconsul.Client, u ttlUpdater, r api.AgentServiceRegistration, logger *adapter.Logger) (sd.Registrar, error) { var ( ttlChecks []ttlCheck err error @@ -148,7 +148,7 @@ func NewRegistrar(c gokitconsul.Client, u ttlUpdater, r *api.AgentServiceRegistr } } - var registrar sd.Registrar = gokitconsul.NewRegistrar(c, r, logger) + var registrar sd.Registrar = gokitconsul.NewRegistrar(c, &r, logger) // decorate the given registrar if we have any TTL checks if len(ttlChecks) > 0 { diff --git a/service/consul/registrar_test.go b/service/consul/registrar_test.go index 2ecad9da..8f103f6a 100644 --- a/service/consul/registrar_test.go +++ b/service/consul/registrar_test.go @@ -39,7 +39,7 @@ func testNewRegistrarNoChecks(t *testing.T) { ttlUpdater = new(mockTTLUpdater) tickerFactory = prepareMockTickerFactory() - registration = &api.AgentServiceRegistration{ + registration = api.AgentServiceRegistration{ ID: "service1", Address: "somehost.com", Port: 1111, @@ -80,7 +80,7 @@ func testNewRegistrarNoTTL(t *testing.T) { ttlUpdater = new(mockTTLUpdater) tickerFactory = prepareMockTickerFactory() - registration = &api.AgentServiceRegistration{ + registration = api.AgentServiceRegistration{ ID: "service1", Address: "somehost.com", Port: 1111, @@ -131,7 +131,7 @@ func testNewRegistrarCheckMalformedTTL(t *testing.T) { ttlUpdater = new(mockTTLUpdater) tickerFactory = prepareMockTickerFactory() - registration = &api.AgentServiceRegistration{ + registration = api.AgentServiceRegistration{ ID: "service1", Address: "somehost.com", Port: 1111, @@ -161,7 +161,7 @@ func testNewRegistrarCheckTTLTooSmall(t *testing.T) { ttlUpdater = new(mockTTLUpdater) tickerFactory = prepareMockTickerFactory() - registration = &api.AgentServiceRegistration{ + registration = api.AgentServiceRegistration{ ID: "service1", Address: "somehost.com", Port: 1111, @@ -191,7 +191,7 @@ func testNewRegistrarChecksMalformedTTL(t *testing.T) { ttlUpdater = new(mockTTLUpdater) tickerFactory = prepareMockTickerFactory() - registration = &api.AgentServiceRegistration{ + registration = api.AgentServiceRegistration{ ID: "service1", Address: "somehost.com", Port: 1111, @@ -223,7 +223,7 @@ func testNewRegistrarChecksTTLTooSmall(t *testing.T) { ttlUpdater = new(mockTTLUpdater) tickerFactory = prepareMockTickerFactory() - registration = &api.AgentServiceRegistration{ + registration = api.AgentServiceRegistration{ ID: "service1", Address: "somehost.com", Port: 1111, @@ -272,7 +272,7 @@ func testNewRegistrarTTL(t *testing.T) { close(update2Done) } - registration = &api.AgentServiceRegistration{ + registration = api.AgentServiceRegistration{ ID: "service1", Address: "somehost.com", Port: 1111,