From be55b0e6275363832948a81b6da1115870114c33 Mon Sep 17 00:00:00 2001 From: Eric Wollesen Date: Tue, 24 Sep 2024 13:36:21 -0600 Subject: [PATCH 1/3] rename the task/test package to task/tasktest This has a few follow-on effects. Namely one no longer needs to have a special name for the task/test package (previously this was custom named to "taskTest"). Now it just follows naturally as "tasktest" with no custom naming required. This change brings the name inline with current Go best practices for package names and package naming, and also makes it easier for LSP-enabled editors to automatically manage Go imports. In addition, it follows patterns found in the Go stdlib (see httptest, slogtest, fstest, iotest, etc.) I also renamed task/test/mock.go to task/tasktest/taskclient_mock_gen.go just to make it extra clear that the file is generated, as well as what it contains (not just a mock, but a mock of the task client). [^1]: https://go.dev/doc/effective_go#package-names --- auth/service/test/service.go | 6 +++--- ehr/reconcile/runner_test.go | 6 +++--- task/service/test/service.go | 6 +++--- task/task.go | 2 +- task/{test => tasktest}/client.go | 2 +- task/{test => tasktest}/task_accessor.go | 2 +- task/{test/mock.go => tasktest/taskclient_mock_gen.go} | 4 ++-- 7 files changed, 14 insertions(+), 14 deletions(-) rename task/{test => tasktest}/client.go (91%) rename task/{test => tasktest}/task_accessor.go (99%) rename task/{test/mock.go => tasktest/taskclient_mock_gen.go} (99%) diff --git a/auth/service/test/service.go b/auth/service/test/service.go index 20f52f1859..543794fe3d 100644 --- a/auth/service/test/service.go +++ b/auth/service/test/service.go @@ -16,7 +16,7 @@ import ( authStoreTest "github.com/tidepool-org/platform/auth/store/test" providerTest "github.com/tidepool-org/platform/provider/test" serviceTest "github.com/tidepool-org/platform/service/test" - taskTest "github.com/tidepool-org/platform/task/test" + "github.com/tidepool-org/platform/task/tasktest" ) type Service struct { @@ -28,7 +28,7 @@ type Service struct { ProviderFactoryInvocations int ProviderFactoryImpl *providerTest.Factory TaskClientInvocations int - TaskClientImpl *taskTest.Client + TaskClientImpl *tasktest.Client StatusInvocations int StatusOutputs []*service.Status confirmationClient confirmationClient.ClientWithResponsesInterface @@ -39,7 +39,7 @@ func NewService() *Service { Service: serviceTest.NewService(), AuthStoreImpl: authStoreTest.NewStore(), ProviderFactoryImpl: providerTest.NewFactory(), - TaskClientImpl: taskTest.NewClient(), + TaskClientImpl: tasktest.NewClient(), } } diff --git a/ehr/reconcile/runner_test.go b/ehr/reconcile/runner_test.go index 118fad8a93..c5c4a1686c 100644 --- a/ehr/reconcile/runner_test.go +++ b/ehr/reconcile/runner_test.go @@ -13,7 +13,7 @@ import ( "github.com/tidepool-org/platform/clinics" "github.com/tidepool-org/platform/log" "github.com/tidepool-org/platform/log/null" - taskTest "github.com/tidepool-org/platform/task/test" + "github.com/tidepool-org/platform/task/tasktest" clinicsTest "github.com/tidepool-org/platform/clinics/test" "github.com/tidepool-org/platform/ehr/reconcile" @@ -29,7 +29,7 @@ var _ = Describe("Runner", func() { var authClient *authTest.MockClient var clinicsClient *clinics.MockClient - var taskClient *taskTest.MockClient + var taskClient *tasktest.MockClient var logger log.Logger BeforeEach(func() { @@ -38,7 +38,7 @@ var _ = Describe("Runner", func() { taskCtrl = gomock.NewController(GinkgoT()) authClient = authTest.NewMockClient(authCtrl) clinicsClient = clinics.NewMockClient(clinicsCtrl) - taskClient = taskTest.NewMockClient(taskCtrl) + taskClient = tasktest.NewMockClient(taskCtrl) logger = null.NewLogger() }) diff --git a/task/service/test/service.go b/task/service/test/service.go index 030141819d..afdcffc08a 100644 --- a/task/service/test/service.go +++ b/task/service/test/service.go @@ -8,7 +8,7 @@ import ( taskService "github.com/tidepool-org/platform/task/service" taskStore "github.com/tidepool-org/platform/task/store" taskStoreTest "github.com/tidepool-org/platform/task/store/test" - taskTest "github.com/tidepool-org/platform/task/test" + "github.com/tidepool-org/platform/task/tasktest" ) type Service struct { @@ -16,7 +16,7 @@ type Service struct { TaskStoreInvocations int TaskStoreImpl *taskStoreTest.Store TaskClientInvocations int - TaskClientImpl *taskTest.Client + TaskClientImpl *tasktest.Client StatusInvocations int StatusOutputs []*taskService.Status } @@ -25,7 +25,7 @@ func NewService() *Service { return &Service{ Service: serviceTest.NewService(), TaskStoreImpl: taskStoreTest.NewStore(), - TaskClientImpl: taskTest.NewClient(), + TaskClientImpl: tasktest.NewClient(), } } diff --git a/task/task.go b/task/task.go index ffda68d4c8..ad6fe72518 100644 --- a/task/task.go +++ b/task/task.go @@ -15,7 +15,7 @@ import ( structureValidator "github.com/tidepool-org/platform/structure/validator" ) -//go:generate mockgen --build_flags=--mod=mod -source=./task.go -destination=./test/mock.go -package test Client +//go:generate mockgen --build_flags=--mod=mod -source=./task.go -destination=./tasktest/taskclient_mock_gen.go -package tasktest Client type Client interface { TaskAccessor } diff --git a/task/test/client.go b/task/tasktest/client.go similarity index 91% rename from task/test/client.go rename to task/tasktest/client.go index f160c98d5f..9e8b91fbad 100644 --- a/task/test/client.go +++ b/task/tasktest/client.go @@ -1,4 +1,4 @@ -package test +package tasktest type Client struct { *TaskAccessor diff --git a/task/test/task_accessor.go b/task/tasktest/task_accessor.go similarity index 99% rename from task/test/task_accessor.go rename to task/tasktest/task_accessor.go index f71b10aa9f..11c1f7fe6a 100644 --- a/task/test/task_accessor.go +++ b/task/tasktest/task_accessor.go @@ -1,4 +1,4 @@ -package test +package tasktest import ( "context" diff --git a/task/test/mock.go b/task/tasktest/taskclient_mock_gen.go similarity index 99% rename from task/test/mock.go rename to task/tasktest/taskclient_mock_gen.go index c702095423..cdd57af808 100644 --- a/task/test/mock.go +++ b/task/tasktest/taskclient_mock_gen.go @@ -1,8 +1,8 @@ // Code generated by MockGen. DO NOT EDIT. // Source: ./task.go -// Package test is a generated GoMock package. -package test +// Package tasktest is a generated GoMock package. +package tasktest import ( context "context" From eaa86262e3a7b22d922930f88a5ef98cb832646e Mon Sep 17 00:00:00 2001 From: Eric Wollesen Date: Tue, 24 Sep 2024 13:59:34 -0600 Subject: [PATCH 2/3] rename the task/store/test package to task/store/taskstoretest This has a few follow-on effects. Namely one no longer needs to have a special name for the task/store/test package (previously this was custom named to "taskStoreTest"). Now it just follows naturally as "taskstoretest" with no custom naming required. This change brings the name inline with current Go best practices for package names and package naming[^1], and also makes it easier for LSP-enabled editors to automatically manage Go imports. In addition, it follows common patterns from the Go stdlib (see httptest, slogtest, fstest, iotest, etc.) In general I would have preferred to keep the name to the concatenation of only two package names, but "store" is a very common package within platform, so I think it makes sense to keep all three parts, as otherwise there would be eight "storetest" packages[^2] and we'd be back to custom renaming the imports which just makes things harder all around. [^1]: https://go.dev/doc/effective_go#package-names [^2]: Yes, eight. To see for yourself: Run `$ go list ./... | rg '/store/' | sed -e 's,/store/.*,/store/,' | sort | uniq`. --- task/service/test/service.go | 6 +++--- task/store/{test => taskstoretest}/store.go | 2 +- task/store/{test => taskstoretest}/task_session.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) rename task/store/{test => taskstoretest}/store.go (97%) rename task/store/{test => taskstoretest}/task_session.go (92%) diff --git a/task/service/test/service.go b/task/service/test/service.go index afdcffc08a..c0617431cf 100644 --- a/task/service/test/service.go +++ b/task/service/test/service.go @@ -7,14 +7,14 @@ import ( "github.com/tidepool-org/platform/task" taskService "github.com/tidepool-org/platform/task/service" taskStore "github.com/tidepool-org/platform/task/store" - taskStoreTest "github.com/tidepool-org/platform/task/store/test" + "github.com/tidepool-org/platform/task/store/taskstoretest" "github.com/tidepool-org/platform/task/tasktest" ) type Service struct { *serviceTest.Service TaskStoreInvocations int - TaskStoreImpl *taskStoreTest.Store + TaskStoreImpl *taskstoretest.Store TaskClientInvocations int TaskClientImpl *tasktest.Client StatusInvocations int @@ -24,7 +24,7 @@ type Service struct { func NewService() *Service { return &Service{ Service: serviceTest.NewService(), - TaskStoreImpl: taskStoreTest.NewStore(), + TaskStoreImpl: taskstoretest.NewStore(), TaskClientImpl: tasktest.NewClient(), } } diff --git a/task/store/test/store.go b/task/store/taskstoretest/store.go similarity index 97% rename from task/store/test/store.go rename to task/store/taskstoretest/store.go index 9ffdd580c7..94f78895c1 100644 --- a/task/store/test/store.go +++ b/task/store/taskstoretest/store.go @@ -1,4 +1,4 @@ -package test +package taskstoretest import ( "context" diff --git a/task/store/test/task_session.go b/task/store/taskstoretest/task_session.go similarity index 92% rename from task/store/test/task_session.go rename to task/store/taskstoretest/task_session.go index 8cb7a8794a..bcceea4c20 100644 --- a/task/store/test/task_session.go +++ b/task/store/taskstoretest/task_session.go @@ -1,4 +1,4 @@ -package test +package taskstoretest import "github.com/tidepool-org/platform/test" From 710b948b9e8afd660ac524a54c306c745ecd1918 Mon Sep 17 00:00:00 2001 From: Eric Wollesen Date: Tue, 24 Sep 2024 14:18:59 -0600 Subject: [PATCH 3/3] rename the task/service/test package to task/service/taskservicetest This has a few follow-on effects. Namely one no longer needs to have a special name for the task/service/test package (previously this was custom named to "taskServiceTest"). Now it just follows naturally as "taskservicetest" with no custom naming required. This change brings the name inline with current Go best practices for package names and package naming, and also makes it easier for LSP-enabled editors to automatically manage Go imports. In addition, it follows patterns found in the Go stdlib (see httptest, slogtest, fstest, iotest, etc.) In general I would have preferred to keep the name to the concatenation of only two package names, but "service" is a very common package within platform, so I think it makes sense to keep all three parts, as otherwise there would be four "servicetest" packages[^2] and we'd be back to custom renaming the imports which just makes things harder all around. [^1]: https://go.dev/doc/effective_go#package-names [^2]: `$ go list ./... | rg '/(task)?service/?test'` --- task/service/api/api_test.go | 8 ++++---- task/service/api/v1/v1_test.go | 6 +++--- task/service/{test => taskservicetest}/service.go | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) rename task/service/{test => taskservicetest}/service.go (98%) diff --git a/task/service/api/api_test.go b/task/service/api/api_test.go index f56b8f9d5a..789333e88f 100644 --- a/task/service/api/api_test.go +++ b/task/service/api/api_test.go @@ -11,15 +11,15 @@ import ( logTest "github.com/tidepool-org/platform/log/test" taskService "github.com/tidepool-org/platform/task/service" taskServiceApi "github.com/tidepool-org/platform/task/service/api" - taskServiceTest "github.com/tidepool-org/platform/task/service/test" + "github.com/tidepool-org/platform/task/service/taskservicetest" testRest "github.com/tidepool-org/platform/test/rest" ) var _ = Describe("API", func() { - var service *taskServiceTest.Service + var service *taskservicetest.Service BeforeEach(func() { - service = taskServiceTest.NewService() + service = taskservicetest.NewService() }) Context("NewRouter", func() { @@ -60,7 +60,7 @@ var _ = Describe("API", func() { response = testRest.NewResponseWriter() request = testRest.NewRequest() request.Request = request.WithContext(log.NewContextWithLogger(request.Context(), logTest.NewLogger())) - service = taskServiceTest.NewService() + service = taskservicetest.NewService() var err error router, err = taskServiceApi.NewRouter(service) Expect(err).ToNot(HaveOccurred()) diff --git a/task/service/api/v1/v1_test.go b/task/service/api/v1/v1_test.go index 94bd3abcb5..abd0a26328 100644 --- a/task/service/api/v1/v1_test.go +++ b/task/service/api/v1/v1_test.go @@ -8,14 +8,14 @@ import ( . "github.com/onsi/gomega/gstruct" taskServiceApiV1 "github.com/tidepool-org/platform/task/service/api/v1" - taskServiceTest "github.com/tidepool-org/platform/task/service/test" + "github.com/tidepool-org/platform/task/service/taskservicetest" ) var _ = Describe("V1", func() { - var service *taskServiceTest.Service + var service *taskservicetest.Service BeforeEach(func() { - service = taskServiceTest.NewService() + service = taskservicetest.NewService() }) Context("NewRouter", func() { diff --git a/task/service/test/service.go b/task/service/taskservicetest/service.go similarity index 98% rename from task/service/test/service.go rename to task/service/taskservicetest/service.go index c0617431cf..2d9652b075 100644 --- a/task/service/test/service.go +++ b/task/service/taskservicetest/service.go @@ -1,4 +1,4 @@ -package test +package taskservicetest import ( "context"