From 239046d6f3eae8af60cccb92a51cc0d93b5c4bea Mon Sep 17 00:00:00 2001 From: Brian Conway Date: Tue, 27 Feb 2024 11:31:33 -0600 Subject: [PATCH] Test updates and style fixes for Go 1.22 for internal/ - All test variations pass. --- LICENSE | 2 +- internal/hermes-api/interceptor/auth_test.go | 24 +++-- internal/hermes-api/interceptor/log_test.go | 16 ++-- .../hermes-api/interceptor/validate_test.go | 12 +-- internal/hermes-api/key/key_test.go | 24 ++--- internal/hermes-api/service/error_test.go | 22 ++--- internal/hermes-api/service/identity.go | 2 +- internal/hermes-api/service/identity_test.go | 10 +- .../hermes-api/session/page_token_test.go | 30 +++--- internal/hermes-api/session/web_token_test.go | 64 ++++++------- internal/hermes-api/test/app_test.go | 2 +- internal/hermes-api/test/event_test.go | 4 +- internal/hermes-api/test/identity_test.go | 8 +- internal/hermes-api/test/org_test.go | 2 +- internal/hermes-api/test/session_test.go | 2 +- internal/hermes-api/test/user_test.go | 2 +- internal/hermes-notifier/notifier/notifier.go | 2 +- .../hermes-notifier/notifier/notify_test.go | 94 +++++++++---------- .../hermes-notifier/template/template_test.go | 14 ++- .../hermes-notifier/test/notifier_test.go | 38 ++++---- 20 files changed, 163 insertions(+), 211 deletions(-) diff --git a/LICENSE b/LICENSE index b5fdab10..634cf288 100644 --- a/LICENSE +++ b/LICENSE @@ -1,3 +1,3 @@ -Copyright (c) 2021-2023 Brian Conway +Copyright (c) 2021-2024 Brian Conway All rights reserved. diff --git a/internal/hermes-api/interceptor/auth_test.go b/internal/hermes-api/interceptor/auth_test.go index 41003b22..857a299e 100644 --- a/internal/hermes-api/interceptor/auth_test.go +++ b/internal/hermes-api/interceptor/auth_test.go @@ -125,39 +125,37 @@ func TestAuth(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can log %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can log %+v", test), func(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) cacher := cache.NewMockCacher(ctrl) cacher.EXPECT().Get(gomock.Any(), gomock.Any()). - Return(lTest.inpCache, "", lTest.inpCacheErr). - Times(lTest.inpCacheTimes) + Return(test.inpCache, "", test.inpCacheErr). + Times(test.inpCacheTimes) orger := service.NewMockOrger(ctrl) - orger.EXPECT().Read(gomock.Any(), lTest.inpOrg.GetId()). - Return(lTest.inpOrg, lTest.inpOrgErr).Times(lTest.inpOrgTimes) + orger.EXPECT().Read(gomock.Any(), test.inpOrg.GetId()). + Return(test.inpOrg, test.inpOrgErr).Times(test.inpOrgTimes) ctx, cancel := context.WithTimeout(context.Background(), testTimeout) defer cancel() - if lTest.inpMD != nil { + if test.inpMD != nil { ctx = metadata.NewIncomingContext(ctx, - metadata.Pairs(lTest.inpMD...)) + metadata.Pairs(test.inpMD...)) } handler := func(_ context.Context, req interface{}) ( interface{}, error, ) { - return req, lTest.inpHandlerErr + return req, test.inpHandlerErr } - res, err := Auth(lTest.inpSkipPaths, key, cacher, orger)(ctx, nil, - lTest.inpInfo, handler) + res, err := Auth(test.inpSkipPaths, key, cacher, orger)(ctx, nil, + test.inpInfo, handler) t.Logf("res, err: %v, %v", res, err) require.Nil(t, res) - require.Equal(t, lTest.err, err) + require.Equal(t, test.err, err) }) } } diff --git a/internal/hermes-api/interceptor/log_test.go b/internal/hermes-api/interceptor/log_test.go index 24b998df..ccb2c204 100644 --- a/internal/hermes-api/interceptor/log_test.go +++ b/internal/hermes-api/interceptor/log_test.go @@ -55,26 +55,24 @@ func TestLog(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can log %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can log %+v", test), func(t *testing.T) { t.Parallel() ctx, cancel := context.WithTimeout(context.Background(), testTimeout) defer cancel() ctx = metadata.NewIncomingContext(ctx, - metadata.Pairs(lTest.inpMD...)) + metadata.Pairs(test.inpMD...)) handler := func(_ context.Context, req any) (any, error) { - return req, lTest.inpHandlerErr + return req, test.inpHandlerErr } - res, err := Log(lTest.inpSkipPaths)(ctx, lTest.inpReq, - lTest.inpInfo, handler) + res, err := Log(test.inpSkipPaths)(ctx, test.inpReq, + test.inpInfo, handler) t.Logf("res, err: %v, %v", res, err) - require.Equal(t, lTest.inpReq, res) - require.Equal(t, lTest.inpHandlerErr, err) + require.Equal(t, test.inpReq, res) + require.Equal(t, test.inpHandlerErr, err) }) } } diff --git a/internal/hermes-api/interceptor/validate_test.go b/internal/hermes-api/interceptor/validate_test.go index 6cddf8e3..a0594625 100644 --- a/internal/hermes-api/interceptor/validate_test.go +++ b/internal/hermes-api/interceptor/validate_test.go @@ -54,9 +54,7 @@ func TestValidate(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can log %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can log %+v", test), func(t *testing.T) { t.Parallel() ctx, cancel := context.WithTimeout(context.Background(), @@ -64,14 +62,14 @@ func TestValidate(t *testing.T) { defer cancel() handler := func(_ context.Context, _ any) (any, error) { - return nil, lTest.err + return nil, test.err } - res, err := Validate(lTest.inpSkipPaths)(ctx, lTest.inpReq, - lTest.inpInfo, handler) + res, err := Validate(test.inpSkipPaths)(ctx, test.inpReq, + test.inpInfo, handler) t.Logf("res, err: %v, %v", res, err) require.Nil(t, res) - require.Equal(t, lTest.err, err) + require.Equal(t, test.err, err) }) } } diff --git a/internal/hermes-api/key/key_test.go b/internal/hermes-api/key/key_test.go index 85de4847..34610d0e 100644 --- a/internal/hermes-api/key/key_test.go +++ b/internal/hermes-api/key/key_test.go @@ -14,10 +14,8 @@ import ( func TestDisabled(t *testing.T) { t.Parallel() - for i := 0; i < 5; i++ { - lTest := i - - t.Run(fmt.Sprintf("Can key %v", lTest), func(t *testing.T) { + for i := range 5 { + t.Run(fmt.Sprintf("Can key %v", i), func(t *testing.T) { t.Parallel() orgID := uuid.NewString() @@ -36,10 +34,8 @@ func TestDisabled(t *testing.T) { func TestTOTPOffset(t *testing.T) { t.Parallel() - for i := 0; i < 5; i++ { - lTest := i - - t.Run(fmt.Sprintf("Can key %v", lTest), func(t *testing.T) { + for i := range 5 { + t.Run(fmt.Sprintf("Can key %v", i), func(t *testing.T) { t.Parallel() orgID := uuid.NewString() @@ -59,10 +55,8 @@ func TestTOTPOffset(t *testing.T) { func TestReuse(t *testing.T) { t.Parallel() - for i := 0; i < 5; i++ { - lTest := i - - t.Run(fmt.Sprintf("Can key %v", lTest), func(t *testing.T) { + for i := range 5 { + t.Run(fmt.Sprintf("Can key %v", i), func(t *testing.T) { t.Parallel() orgID := uuid.NewString() @@ -83,10 +77,8 @@ func TestReuse(t *testing.T) { func TestChallenge(t *testing.T) { t.Parallel() - for i := 0; i < 5; i++ { - lTest := i - - t.Run(fmt.Sprintf("Can key %v", lTest), func(t *testing.T) { + for i := range 5 { + t.Run(fmt.Sprintf("Can key %v", i), func(t *testing.T) { t.Parallel() orgID := uuid.NewString() diff --git a/internal/hermes-api/service/error_test.go b/internal/hermes-api/service/error_test.go index f34d453e..9a8a49ef 100644 --- a/internal/hermes-api/service/error_test.go +++ b/internal/hermes-api/service/error_test.go @@ -43,19 +43,17 @@ func TestErrToStatus(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can map %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can map %+v", test), func(t *testing.T) { t.Parallel() - res := errToStatus(lTest.inp) + res := errToStatus(test.inp) t.Logf("res: %#v", res) // Comparison of gRPC status errors does not play well with // require.Equal. - if lTest.res == "" { + if test.res == "" { require.NoError(t, res) } else { - require.EqualError(t, res, lTest.res) + require.EqualError(t, res, test.res) } }) } @@ -64,10 +62,8 @@ func TestErrToStatus(t *testing.T) { func TestErrPerm(t *testing.T) { t.Parallel() - for i := 0; i < 5; i++ { - lTest := i - - t.Run(fmt.Sprintf("Can generate %v", lTest), func(t *testing.T) { + for i := range 5 { + t.Run(fmt.Sprintf("Can generate %v", i), func(t *testing.T) { t.Parallel() role := []api.Role{ @@ -90,10 +86,8 @@ func TestErrPerm(t *testing.T) { func TestErrPlan(t *testing.T) { t.Parallel() - for i := 0; i < 5; i++ { - lTest := i - - t.Run(fmt.Sprintf("Can generate %v", lTest), func(t *testing.T) { + for i := range 5 { + t.Run(fmt.Sprintf("Can generate %v", i), func(t *testing.T) { t.Parallel() plan := []api.Plan{ diff --git a/internal/hermes-api/service/identity.go b/internal/hermes-api/service/identity.go index 97107aba..d48dc302 100644 --- a/internal/hermes-api/service/identity.go +++ b/internal/hermes-api/service/identity.go @@ -135,7 +135,7 @@ func (ai *AppIdentity) CreateIdentity( // Populate pregenerated backup codes. if m, ok := identity.GetMethodOneof().(*api.Identity_BackupCodesMethod); ok { - for i := 0; i < int(m.BackupCodesMethod.GetPasscodes()); i++ { + for i := range m.BackupCodesMethod.GetPasscodes() { passcode, err := otp.HOTP(int64(i + 10)) if err != nil { return nil, errToStatus(err) diff --git a/internal/hermes-api/service/identity_test.go b/internal/hermes-api/service/identity_test.go index 6d66e8be..3a503818 100644 --- a/internal/hermes-api/service/identity_test.go +++ b/internal/hermes-api/service/identity_test.go @@ -438,22 +438,20 @@ func TestCreateIdentity(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can create %v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can create %v", test), func(t *testing.T) { t.Parallel() ctx, cancel := context.WithTimeout(session.NewContext( context.Background(), &session.Session{ - OrgID: lTest.GetOrgId(), OrgPlan: api.Plan_STARTER, + OrgID: test.GetOrgId(), OrgPlan: api.Plan_STARTER, Role: api.Role_ADMIN, }), testTimeout) defer cancel() aiSvc := NewAppIdentity(nil, nil, nil, nil, nil, nil, "") createIdentity, err := aiSvc.CreateIdentity(ctx, - &api.CreateIdentityRequest{Identity: lTest}) - t.Logf("lTest, createIdentity, err: %+v, %+v, %v", lTest, + &api.CreateIdentityRequest{Identity: test}) + t.Logf("test, createIdentity, err: %+v, %+v, %v", test, createIdentity, err) require.Nil(t, createIdentity) require.Equal(t, errPlan(api.Plan_PRO), err) diff --git a/internal/hermes-api/session/page_token_test.go b/internal/hermes-api/session/page_token_test.go index a2f1e4e2..f19bd6f3 100644 --- a/internal/hermes-api/session/page_token_test.go +++ b/internal/hermes-api/session/page_token_test.go @@ -30,18 +30,16 @@ func TestGeneratePageToken(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can generate %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can generate %+v", test), func(t *testing.T) { t.Parallel() - res, err := GeneratePageToken(lTest.inpTS, lTest.inpID) + res, err := GeneratePageToken(test.inpTS, test.inpID) t.Logf("res, err: %v, %#v", res, err) - require.GreaterOrEqual(t, len(res), lTest.resMinLen) - if lTest.err == "" { + require.GreaterOrEqual(t, len(res), test.resMinLen) + if test.err == "" { require.NoError(t, err) } else { - require.EqualError(t, err, lTest.err) + require.EqualError(t, err, test.err) } }) } @@ -93,31 +91,29 @@ func TestParsePageToken(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can generate %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can generate %+v", test), func(t *testing.T) { t.Parallel() - resGen, err := GeneratePageToken(lTest.inpTS, lTest.inpID) + resGen, err := GeneratePageToken(test.inpTS, test.inpID) t.Logf("resGen, err: %v, %#v", resGen, err) require.NoError(t, err) var resTS time.Time var resID string - if lTest.inpPT == "res" { + if test.inpPT == "res" { resTS, resID, err = ParsePageToken(resGen) } else { - resTS, resID, err = ParsePageToken(lTest.inpPT) + resTS, resID, err = ParsePageToken(test.inpPT) } t.Logf("resTS, resID, err: %v, %v, %#v", resTS, resID, err) if resID != "" { - require.Equal(t, lTest.inpID, resID) - require.Equal(t, lTest.inpTS, resTS) + require.Equal(t, test.inpID, resID) + require.Equal(t, test.inpTS, resTS) } - if lTest.err == "" { + if test.err == "" { require.NoError(t, err) } else { - require.Contains(t, err.Error(), lTest.err) + require.Contains(t, err.Error(), test.err) } }) } diff --git a/internal/hermes-api/session/web_token_test.go b/internal/hermes-api/session/web_token_test.go index e3c10166..f3398638 100644 --- a/internal/hermes-api/session/web_token_test.go +++ b/internal/hermes-api/session/web_token_test.go @@ -52,25 +52,23 @@ func TestGenerateWebToken(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can generate %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can generate %+v", test), func(t *testing.T) { t.Parallel() - res, exp, err := GenerateWebToken(lTest.inpKey, &api.User{ - Id: lTest.inpUserID, OrgId: lTest.inpOrgID, + res, exp, err := GenerateWebToken(test.inpKey, &api.User{ + Id: test.inpUserID, OrgId: test.inpOrgID, Role: api.Role_AUTHENTICATOR, }) t.Logf("res, exp, err: %v, %+v, %#v", res, exp, err) - require.GreaterOrEqual(t, len(res), lTest.resMinLen) + require.GreaterOrEqual(t, len(res), test.resMinLen) if exp != nil { require.WithinDuration(t, time.Now().Add( WebTokenExp*time.Second), exp.AsTime(), 2*time.Second) } - if lTest.err == "" { + if test.err == "" { require.NoError(t, err) } else { - require.EqualError(t, err, lTest.err) + require.EqualError(t, err, test.err) } }) } @@ -109,19 +107,17 @@ func TestGenerateKeyToken(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can generate %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can generate %+v", test), func(t *testing.T) { t.Parallel() - res, err := GenerateKeyToken(lTest.inpKey, lTest.inpKeyID, - lTest.inpOrgID, api.Role_AUTHENTICATOR) + res, err := GenerateKeyToken(test.inpKey, test.inpKeyID, + test.inpOrgID, api.Role_AUTHENTICATOR) t.Logf("res, err: %v, %#v", res, err) - require.GreaterOrEqual(t, len(res), lTest.resMinLen) - if lTest.err == "" { + require.GreaterOrEqual(t, len(res), test.resMinLen) + if test.err == "" { require.NoError(t, err) } else { - require.EqualError(t, err, lTest.err) + require.EqualError(t, err, test.err) } }) } @@ -169,22 +165,20 @@ func TestValidateWebToken(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can validate %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can validate %+v", test), func(t *testing.T) { t.Parallel() user := random.User("webtoken", uuid.NewString()) - resGen, exp, err := GenerateWebToken(lTest.inpKey, user) + resGen, exp, err := GenerateWebToken(test.inpKey, user) t.Logf("resGen, exp, err: %v, %v, %v", resGen, exp, err) require.NoError(t, err) var resVal *Session - if lTest.inpCiphertoken == "" { - resVal, err = ValidateWebToken(lTest.inpKey, resGen) + if test.inpCiphertoken == "" { + resVal, err = ValidateWebToken(test.inpKey, resGen) } else { - resVal, err = ValidateWebToken(lTest.inpKey, - lTest.inpCiphertoken) + resVal, err = ValidateWebToken(test.inpKey, + test.inpCiphertoken) } t.Logf("resVal, err: %+v, %v", resVal, err) if resVal != nil { @@ -194,10 +188,10 @@ func TestValidateWebToken(t *testing.T) { require.Equal(t, user.GetRole(), resVal.Role) require.NotEmpty(t, resVal.TraceID) } - if lTest.err == "" { + if test.err == "" { require.NoError(t, err) } else { - require.Contains(t, err.Error(), lTest.err) + require.Contains(t, err.Error(), test.err) } }) } @@ -234,25 +228,23 @@ func TestValidateKeyToken(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can validate %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can validate %+v", test), func(t *testing.T) { t.Parallel() keyID := uuid.NewString() user := random.User("keytoken", uuid.NewString()) - resGen, err := GenerateKeyToken(lTest.inpKey, keyID, user.GetOrgId(), + resGen, err := GenerateKeyToken(test.inpKey, keyID, user.GetOrgId(), user.GetRole()) t.Logf("resGen, err: %v, %v", resGen, err) require.NoError(t, err) var resVal *Session - if lTest.inpCiphertoken == "" { - resVal, err = ValidateWebToken(lTest.inpKey, resGen) + if test.inpCiphertoken == "" { + resVal, err = ValidateWebToken(test.inpKey, resGen) } else { - resVal, err = ValidateWebToken(lTest.inpKey, - lTest.inpCiphertoken) + resVal, err = ValidateWebToken(test.inpKey, + test.inpCiphertoken) } t.Logf("resVal, err: %+v, %v", resVal, err) if resVal != nil { @@ -262,10 +254,10 @@ func TestValidateKeyToken(t *testing.T) { require.Equal(t, user.GetRole(), resVal.Role) require.NotEmpty(t, resVal.TraceID) } - if lTest.err == "" { + if test.err == "" { require.NoError(t, err) } else { - require.Contains(t, err.Error(), lTest.err) + require.Contains(t, err.Error(), test.err) } }) } diff --git a/internal/hermes-api/test/app_test.go b/internal/hermes-api/test/app_test.go index 82176f8f..c8c173d3 100644 --- a/internal/hermes-api/test/app_test.go +++ b/internal/hermes-api/test/app_test.go @@ -490,7 +490,7 @@ func TestListApps(t *testing.T) { appIDs := []string{} appNames := []string{} - for i := 0; i < 3; i++ { + for range 3 { app := random.App("api-app", uuid.NewString()) aiCli := api.NewAppIdentityServiceClient(globalAdminGRPCConn) diff --git a/internal/hermes-api/test/event_test.go b/internal/hermes-api/test/event_test.go index 2189569c..c9739c26 100644 --- a/internal/hermes-api/test/event_test.go +++ b/internal/hermes-api/test/event_test.go @@ -42,7 +42,7 @@ func TestListEvents(t *testing.T) { events := []*api.Event{} - for i := 0; i < 5; i++ { + for range 5 { event := random.Event("api-event", globalAdminOrgID) event.AppId = createApp.GetId() event.IdentityId = createIdentity.GetIdentity().GetId() @@ -170,7 +170,7 @@ func TestLatestEvents(t *testing.T) { events := []*api.Event{} - for i := 0; i < 5; i++ { + for range 5 { event := random.Event("api-event", globalAdminOrgID) events = append(events, event) diff --git a/internal/hermes-api/test/identity_test.go b/internal/hermes-api/test/identity_test.go index 7179dc55..20a834f0 100644 --- a/internal/hermes-api/test/identity_test.go +++ b/internal/hermes-api/test/identity_test.go @@ -277,9 +277,7 @@ func TestCreateIdentity(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can create %v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can create %v", test), func(t *testing.T) { t.Parallel() ctx, cancel := context.WithTimeout(context.Background(), @@ -288,7 +286,7 @@ func TestCreateIdentity(t *testing.T) { aiCli := api.NewAppIdentityServiceClient(adminStarterGRPCConn) createIdentity, err := aiCli.CreateIdentity(ctx, - &api.CreateIdentityRequest{Identity: lTest}) + &api.CreateIdentityRequest{Identity: test}) t.Logf("createIdentity, err: %+v, %v", createIdentity, err) require.Nil(t, createIdentity) require.EqualError(t, err, "rpc error: code = "+ @@ -1781,7 +1779,7 @@ func TestListIdentities(t *testing.T) { identityIDs := []string{} identityComments := []string{} - for i := 0; i < 3; i++ { + for range 3 { identity := random.HOTPIdentity("api-identity", uuid.NewString(), createApp.GetId()) diff --git a/internal/hermes-api/test/org_test.go b/internal/hermes-api/test/org_test.go index bd55f32a..f2750e22 100644 --- a/internal/hermes-api/test/org_test.go +++ b/internal/hermes-api/test/org_test.go @@ -461,7 +461,7 @@ func TestListOrgs(t *testing.T) { orgIDs := []string{} orgNames := []string{} orgPlans := []api.Plan{} - for i := 0; i < 3; i++ { + for range 3 { orgCli := api.NewOrgServiceClient(secondarySysAdminGRPCConn) createOrg, err := orgCli.CreateOrg(ctx, &api.CreateOrgRequest{Org: random.Org("api-org")}) diff --git a/internal/hermes-api/test/session_test.go b/internal/hermes-api/test/session_test.go index e1fef744..946581c5 100644 --- a/internal/hermes-api/test/session_test.go +++ b/internal/hermes-api/test/session_test.go @@ -396,7 +396,7 @@ func TestListKeys(t *testing.T) { keyIDs := []string{} keyNames := []string{} keyRoles := []api.Role{} - for i := 0; i < 3; i++ { + for range 3 { key := random.Key("api-key", uuid.NewString()) key.Role = api.Role_AUTHENTICATOR diff --git a/internal/hermes-api/test/user_test.go b/internal/hermes-api/test/user_test.go index 3754159c..c87761dd 100644 --- a/internal/hermes-api/test/user_test.go +++ b/internal/hermes-api/test/user_test.go @@ -729,7 +729,7 @@ func TestListUsers(t *testing.T) { userNames := []string{} userRoles := []api.Role{} userStatuses := []api.Status{} - for i := 0; i < 3; i++ { + for range 3 { user := random.User("api-user", uuid.NewString()) user.Role = api.Role_AUTHENTICATOR diff --git a/internal/hermes-notifier/notifier/notifier.go b/internal/hermes-notifier/notifier/notifier.go index eba63c40..64206218 100644 --- a/internal/hermes-notifier/notifier/notifier.go +++ b/internal/hermes-notifier/notifier/notifier.go @@ -126,7 +126,7 @@ func New(cfg *config.Config) (*Notifier, error) { // Serve starts the message notifiers. func (not *Notifier) Serve(concurrency int) { - for i := 0; i < concurrency; i++ { + for range concurrency { go not.notifyMessages() } diff --git a/internal/hermes-notifier/notifier/notify_test.go b/internal/hermes-notifier/notifier/notify_test.go index fdc14b2d..1ddb8f6f 100644 --- a/internal/hermes-notifier/notifier/notify_test.go +++ b/internal/hermes-notifier/notifier/notify_test.go @@ -82,9 +82,7 @@ func TestNotifyMessages(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can notify %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can notify %+v", test), func(t *testing.T) { t.Parallel() nInQueue := queue.NewFake() @@ -101,38 +99,38 @@ func TestNotifyMessages(t *testing.T) { ctrl := gomock.NewController(t) identityer := NewMockidentityer(ctrl) - identityer.EXPECT().Read(gomock.Any(), lTest.inpNIn.GetIdentityId(), - lTest.inpNIn.GetOrgId(), lTest.inpNIn.GetAppId()). - Return(lTest.inpIdentity, otp, nil).Times(1) + identityer.EXPECT().Read(gomock.Any(), test.inpNIn.GetIdentityId(), + test.inpNIn.GetOrgId(), test.inpNIn.GetAppId()). + Return(test.inpIdentity, otp, nil).Times(1) cacher := cache.NewMockCacher(ctrl) cacher.EXPECT().Incr(gomock.Any(), key.HOTPCounter( - lTest.inpNIn.GetOrgId(), lTest.inpNIn.GetAppId(), - lTest.inpNIn.GetIdentityId())).Return(int64(5), nil).Times(1) + test.inpNIn.GetOrgId(), test.inpNIn.GetAppId(), + test.inpNIn.GetIdentityId())).Return(int64(5), nil).Times(1) cacher.EXPECT().SetIfNotExistTTL(gomock.Any(), key.Expire( - lTest.inpNIn.GetOrgId(), lTest.inpNIn.GetAppId(), lTest.inpNIn.GetIdentityId(), - "861821"), 1, lTest.inpExpire).Return(true, nil).Times(1) + test.inpNIn.GetOrgId(), test.inpNIn.GetAppId(), test.inpNIn.GetIdentityId(), + "861821"), 1, test.inpExpire).Return(true, nil).Times(1) apper := NewMockapper(ctrl) - apper.EXPECT().Read(gomock.Any(), lTest.inpNIn.GetAppId(), - lTest.inpNIn.GetOrgId()).Return(lTest.inpApp, nil).Times(1) + apper.EXPECT().Read(gomock.Any(), test.inpNIn.GetAppId(), + test.inpNIn.GetOrgId()).Return(test.inpApp, nil).Times(1) notifier := notify.NewMockNotifier(ctrl) notifier.EXPECT().SMS(gomock.Any(), - smsIdentity.GetSmsMethod().GetPhone(), lTest.inpApp.GetDisplayName(), - "861821").Return(nil).Times(lTest.inpSMSTimes) + smsIdentity.GetSmsMethod().GetPhone(), test.inpApp.GetDisplayName(), + "861821").Return(nil).Times(test.inpSMSTimes) notifier.EXPECT().Pushover(gomock.Any(), pushoverIdentity.GetPushoverMethod().GetPushoverKey(), - lTest.inpApp.GetDisplayName(), "861821").Return(nil). - Times(lTest.inpPushoverTimes) + test.inpApp.GetDisplayName(), "861821").Return(nil). + Times(test.inpPushoverTimes) notifier.EXPECT().PushoverByApp(gomock.Any(), - lTest.inpApp.GetPushoverKey(), + test.inpApp.GetPushoverKey(), identityByKey.GetPushoverMethod().GetPushoverKey(), gomock.Any(), - gomock.Any()).Return(nil).Times(lTest.inpPushoverByAppTimes) - notifier.EXPECT().Email(gomock.Any(), lTest.inpApp.GetDisplayName(), - lTest.inpApp.GetEmail(), emailIdentity.GetEmailMethod().GetEmail(), + gomock.Any()).Return(nil).Times(test.inpPushoverByAppTimes) + notifier.EXPECT().Email(gomock.Any(), test.inpApp.GetDisplayName(), + test.inpApp.GetEmail(), emailIdentity.GetEmailMethod().GetEmail(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil). - Times(lTest.inpEmailTimes) + Times(test.inpEmailTimes) eventer := NewMockeventer(ctrl) eventer.EXPECT().Create(gomock.Any(), gomock.Any()). @@ -157,7 +155,7 @@ func TestNotifyMessages(t *testing.T) { not.notifyMessages() }() - bNIn, err := proto.Marshal(lTest.inpNIn) + bNIn, err := proto.Marshal(test.inpNIn) require.NoError(t, err) t.Logf("bNIn: %s", bNIn) @@ -263,9 +261,7 @@ func TestNotifyMessagesError(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can notify %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can notify %+v", test), func(t *testing.T) { t.Parallel() nInQueue := queue.NewFake() @@ -278,20 +274,20 @@ func TestNotifyMessagesError(t *testing.T) { ctrl := gomock.NewController(t) identityer := NewMockidentityer(ctrl) identityer.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), - gomock.Any()).Return(lTest.inpIdentity, lTest.inpOTP, - lTest.inpIdentityErr).Times(lTest.inpIdentityTimes) + gomock.Any()).Return(test.inpIdentity, test.inpOTP, + test.inpIdentityErr).Times(test.inpIdentityTimes) cacher := cache.NewMockCacher(ctrl) cacher.EXPECT().Incr(gomock.Any(), gomock.Any()).Return(int64(5), - lTest.inpIncrErr).Times(lTest.inpIncrTimes) + test.inpIncrErr).Times(test.inpIncrTimes) apper := NewMockapper(ctrl) apper.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any()). - Return(lTest.inpApp, lTest.inpAppErr).Times(lTest.inpAppTimes) + Return(test.inpApp, test.inpAppErr).Times(test.inpAppTimes) cacher.EXPECT().SetIfNotExistTTL(gomock.Any(), gomock.Any(), 1, - lTest.inpExpire).Return(true, lTest.inpSetIfNotExistTTLErr). - Times(lTest.inpSetIfNotExistTTLTimes) + test.inpExpire).Return(true, test.inpSetIfNotExistTTLErr). + Times(test.inpSetIfNotExistTTLTimes) notifier := notify.NewMockNotifier(ctrl) notifier.EXPECT().SMS(gomock.Any(), @@ -299,8 +295,8 @@ func TestNotifyMessagesError(t *testing.T) { DoAndReturn(func(_ any, _ any, _ any, _ any) error { defer wg.Done() - return lTest.inpSMSErr - }).Times(lTest.inpSMSTimes) + return test.inpSMSErr + }).Times(test.inpSMSTimes) notifier.EXPECT().Pushover(gomock.Any(), pushoverIdentity.GetPushoverMethod().GetPushoverKey(), app.GetDisplayName(), "861821").DoAndReturn(func( @@ -308,8 +304,8 @@ func TestNotifyMessagesError(t *testing.T) { ) error { defer wg.Done() - return lTest.inpPushoverErr - }).Times(lTest.inpPushoverTimes) + return test.inpPushoverErr + }).Times(test.inpPushoverTimes) notifier.EXPECT().Email(gomock.Any(), app.GetDisplayName(), app.GetEmail(), emailIdentity.GetEmailMethod().GetEmail(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func( @@ -317,8 +313,8 @@ func TestNotifyMessagesError(t *testing.T) { ) error { defer wg.Done() - return lTest.inpEmailErr - }).Times(lTest.inpEmailTimes) + return test.inpEmailErr + }).Times(test.inpEmailTimes) eventer := NewMockeventer(ctrl) eventer.EXPECT().Create(gomock.Any(), gomock.Any()). @@ -340,14 +336,14 @@ func TestNotifyMessagesError(t *testing.T) { }() bNIn := []byte("not-aaa") - if lTest.inpNIn != nil { - bNIn, err = proto.Marshal(lTest.inpNIn) + if test.inpNIn != nil { + bNIn, err = proto.Marshal(test.inpNIn) require.NoError(t, err) t.Logf("bNIn: %s", bNIn) } require.NoError(t, nInQueue.Publish("", bNIn)) - if lTest.inpNotifyTimes > 0 { + if test.inpNotifyTimes > 0 { wg.Wait() } else { // If the success mode isn't supported by WaitGroup operation, @@ -387,22 +383,20 @@ func TestGenTemplates(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can generate %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can generate %+v", test), func(t *testing.T) { t.Parallel() - subj, body, htmlBody, err := genTemplates(lTest.inpApp, - lTest.inpPasscode) + subj, body, htmlBody, err := genTemplates(test.inpApp, + test.inpPasscode) t.Logf("subj, body, htmlBody, err: %v, %v, %v, %v", subj, body, htmlBody, err) - require.Equal(t, lTest.resSubj, subj) - require.Equal(t, lTest.resBody, body) - require.Equal(t, lTest.resHTMLBody, htmlBody) - if lTest.err == "" { + require.Equal(t, test.resSubj, subj) + require.Equal(t, test.resBody, body) + require.Equal(t, test.resHTMLBody, htmlBody) + if test.err == "" { require.NoError(t, err) } else { - require.Contains(t, err.Error(), lTest.err) + require.Contains(t, err.Error(), test.err) } }) } diff --git a/internal/hermes-notifier/template/template_test.go b/internal/hermes-notifier/template/template_test.go index dca9e5f3..12e6f5d4 100644 --- a/internal/hermes-notifier/template/template_test.go +++ b/internal/hermes-notifier/template/template_test.go @@ -36,19 +36,17 @@ func TestGenerate(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can generate %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can generate %+v", test), func(t *testing.T) { t.Parallel() - res, err := Generate(lTest.inpDisplayName, lTest.inpPasscode, - lTest.inpTempl) + res, err := Generate(test.inpDisplayName, test.inpPasscode, + test.inpTempl) t.Logf("res, err: %v, %#v", res, err) - require.Equal(t, lTest.res, res) - if lTest.err == "" { + require.Equal(t, test.res, res) + if test.err == "" { require.NoError(t, err) } else { - require.Contains(t, err.Error(), lTest.err) + require.Contains(t, err.Error(), test.err) } }) } diff --git a/internal/hermes-notifier/test/notifier_test.go b/internal/hermes-notifier/test/notifier_test.go index 7ef66606..0c3356ce 100644 --- a/internal/hermes-notifier/test/notifier_test.go +++ b/internal/hermes-notifier/test/notifier_test.go @@ -93,12 +93,10 @@ func TestNotifyMessages(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Can notify %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Can notify %+v", test), func(t *testing.T) { t.Parallel() - bNIn, err := proto.Marshal(lTest.inp) + bNIn, err := proto.Marshal(test.inp) require.NoError(t, err) t.Logf("bNIn: %s", bNIn) @@ -107,9 +105,9 @@ func TestNotifyMessages(t *testing.T) { // Verify event. event := &api.Event{ - OrgId: lTest.inp.GetOrgId(), - AppId: lTest.inp.GetAppId(), - IdentityId: lTest.inp.GetIdentityId(), + OrgId: test.inp.GetOrgId(), + AppId: test.inp.GetAppId(), + IdentityId: test.inp.GetIdentityId(), Status: api.EventStatus_CHALLENGE_SENT, TraceId: traceID.String(), } @@ -118,8 +116,8 @@ func TestNotifyMessages(t *testing.T) { testTimeout) defer cancel() - listEvents, err := globalEvDAO.List(ctx, lTest.inp.GetOrgId(), - lTest.inp.GetIdentityId(), time.Now(), time.Now().Add(-testTimeout)) + listEvents, err := globalEvDAO.List(ctx, test.inp.GetOrgId(), + test.inp.GetIdentityId(), time.Now(), time.Now().Add(-testTimeout)) t.Logf("listEvents, err: %+v, %v", listEvents, err) require.NoError(t, err) require.Len(t, listEvents, 1) @@ -218,31 +216,29 @@ func TestNotifyMessagesError(t *testing.T) { } for _, test := range tests { - lTest := test - - t.Run(fmt.Sprintf("Cannot notify %+v", lTest), func(t *testing.T) { + t.Run(fmt.Sprintf("Cannot notify %+v", test), func(t *testing.T) { t.Parallel() bNIn := []byte("not-aaa") - if lTest.inpNIn != nil { + if test.inpNIn != nil { var err error - bNIn, err = proto.Marshal(lTest.inpNIn) + bNIn, err = proto.Marshal(test.inpNIn) require.NoError(t, err) t.Logf("bNIn: %s", bNIn) } require.NoError(t, globalNotQueue.Publish(globalNInSubTopic, bNIn)) - if lTest.inpEvent { + if test.inpEvent { time.Sleep(2 * time.Second) // Verify event. event := &api.Event{ - OrgId: lTest.inpNIn.GetOrgId(), - AppId: lTest.inpNIn.GetAppId(), - IdentityId: lTest.inpNIn.GetIdentityId(), + OrgId: test.inpNIn.GetOrgId(), + AppId: test.inpNIn.GetAppId(), + IdentityId: test.inpNIn.GetIdentityId(), Status: api.EventStatus_CHALLENGE_FAIL, - Error: lTest.inpEventErr, + Error: test.inpEventErr, TraceId: traceID.String(), } @@ -250,8 +246,8 @@ func TestNotifyMessagesError(t *testing.T) { testTimeout) defer cancel() - listEvents, err := globalEvDAO.List(ctx, lTest.inpNIn.GetOrgId(), - lTest.inpNIn.GetIdentityId(), time.Now(), + listEvents, err := globalEvDAO.List(ctx, test.inpNIn.GetOrgId(), + test.inpNIn.GetIdentityId(), time.Now(), time.Now().Add(-testTimeout)) t.Logf("listEvents, err: %+v, %v", listEvents, err) require.NoError(t, err)