From fdb161cacb24f7c457664fee924ec9f624219096 Mon Sep 17 00:00:00 2001 From: Pavel <177363085+pkcll@users.noreply.github.com> Date: Fri, 24 Jul 2026 11:44:42 -0400 Subject: [PATCH 1/2] feat: add gofix Makefile target and CI check --- .github/workflows/pkg.yml | 20 ++++++++++++++ Makefile | 4 +++ pkg/beholder/batch_emitter_service_test.go | 31 +++++++++++----------- pkg/beholder/client_test.go | 3 ++- 4 files changed, 41 insertions(+), 17 deletions(-) diff --git a/.github/workflows/pkg.yml b/.github/workflows/pkg.yml index 769f359377..deca932e27 100644 --- a/.github/workflows/pkg.yml +++ b/.github/workflows/pkg.yml @@ -96,6 +96,26 @@ jobs: path: | ./race.* + check-gofix: + runs-on: ubuntu-latest + # Non-required while we evaluate it; failure won't block merges. + continue-on-error: true + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Go + uses: ./.github/actions/setup-go + with: + go-version-file: "go.mod" + only-modules: "true" + + - name: Ensure "make gofix" has been run + run: | + make gofix + git add --all + git diff --minimal --cached --exit-code + check-tidy: runs-on: ubuntu-latest steps: diff --git a/Makefile b/Makefile index 90f8ccddee..62a53c3d52 100644 --- a/Makefile +++ b/Makefile @@ -60,6 +60,10 @@ modgraph: gomods go install github.com/jmank88/modgraph@v0.1.0 ./modgraph > go.md +.PHONY: gofix +gofix: ## Run go fix across all packages + go fix ./... + .PHONY: dependabot ifndef DEPENDABOT_SEVERITY DEPENDABOT_SEVERITY := "critical,high" diff --git a/pkg/beholder/batch_emitter_service_test.go b/pkg/beholder/batch_emitter_service_test.go index 5a405a6b9b..0ec235863d 100644 --- a/pkg/beholder/batch_emitter_service_test.go +++ b/pkg/beholder/batch_emitter_service_test.go @@ -49,13 +49,13 @@ func TestNewChipIngressBatchEmitterService(t *testing.T) { t.Run("happy path", func(t *testing.T) { clientMock := mocks.NewClient(t) clientMock.EXPECT().Close().Return(nil).Maybe() - emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, newTestConfig(), newTestLogger(t)) + emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, newTestConfig(), logger.Test(t)) require.NoError(t, err) assert.NotNil(t, emitter) }) t.Run("returns error when client is nil", func(t *testing.T) { - emitter, err := beholder.NewChipIngressBatchEmitterService(nil, newTestConfig(), newTestLogger(t)) + emitter, err := beholder.NewChipIngressBatchEmitterService(nil, newTestConfig(), logger.Test(t)) assert.Error(t, err) assert.Nil(t, emitter) }) @@ -65,7 +65,7 @@ func TestChipIngressBatchEmitterService_Emit(t *testing.T) { t.Run("returns error when domain/entity missing", func(t *testing.T) { clientMock := mocks.NewClient(t) clientMock.EXPECT().Close().Return(nil).Maybe() - emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, newTestConfig(), newTestLogger(t)) + emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, newTestConfig(), logger.Test(t)) require.NoError(t, err) require.NoError(t, emitter.Start(t.Context())) defer emitter.Close() //nolint:errcheck @@ -93,7 +93,7 @@ func TestChipIngressBatchEmitterService_Emit(t *testing.T) { cfg := newTestConfig() cfg.ChipIngressSendInterval = 50 * time.Millisecond - emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, newTestLogger(t)) + emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, logger.Test(t)) require.NoError(t, err) require.NoError(t, emitter.Start(t.Context())) @@ -142,7 +142,7 @@ func TestChipIngressBatchEmitterService_CloudEventFormat(t *testing.T) { cfg := newTestConfig() cfg.ChipIngressSendInterval = 50 * time.Millisecond - emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, newTestLogger(t)) + emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, logger.Test(t)) require.NoError(t, err) require.NoError(t, emitter.Start(t.Context())) @@ -188,7 +188,7 @@ func TestChipIngressBatchEmitterService_PublishBatchError(t *testing.T) { cfg := newTestConfig() cfg.ChipIngressSendInterval = 50 * time.Millisecond - emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, newTestLogger(t)) + emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, logger.Test(t)) require.NoError(t, err) require.NoError(t, emitter.Start(t.Context())) @@ -221,7 +221,7 @@ func TestChipIngressBatchEmitterService_ContextCancellation(t *testing.T) { cfg.ChipIngressBufferSize = 1 cfg.ChipIngressSendInterval = 10 * time.Second - emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, newTestLogger(t)) + emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, logger.Test(t)) require.NoError(t, err) require.NoError(t, emitter.Start(t.Context())) defer emitter.Close() //nolint:errcheck @@ -251,7 +251,7 @@ func TestChipIngressBatchEmitterService_DefaultConfig(t *testing.T) { }). Return(nil, nil) - emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, beholder.Config{}, newTestLogger(t)) + emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, beholder.Config{}, logger.Test(t)) require.NoError(t, err) require.NoError(t, emitter.Start(t.Context())) @@ -282,7 +282,7 @@ func TestChipIngressBatchEmitterService_EmitAfterClose(t *testing.T) { Return(nil, nil). Maybe() - emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, newTestConfig(), newTestLogger(t)) + emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, newTestConfig(), logger.Test(t)) require.NoError(t, err) require.NoError(t, emitter.Start(t.Context())) require.NoError(t, emitter.Close()) @@ -305,7 +305,7 @@ func TestChipIngressBatchEmitterService_EmitWithCallback(t *testing.T) { cfg := newTestConfig() cfg.ChipIngressSendInterval = 50 * time.Millisecond - emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, newTestLogger(t)) + emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, logger.Test(t)) require.NoError(t, err) require.NoError(t, emitter.Start(t.Context())) @@ -338,7 +338,7 @@ func TestChipIngressBatchEmitterService_EmitWithCallback(t *testing.T) { cfg := newTestConfig() cfg.ChipIngressSendInterval = 50 * time.Millisecond - emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, newTestLogger(t)) + emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, logger.Test(t)) require.NoError(t, err) require.NoError(t, emitter.Start(t.Context())) @@ -447,7 +447,7 @@ func TestChipIngressBatchEmitterService_EmitWithCallback(t *testing.T) { cfg := newTestConfig() cfg.ChipIngressSendInterval = 50 * time.Millisecond - emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, newTestLogger(t)) + emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, logger.Test(t)) require.NoError(t, err) require.NoError(t, emitter.Start(t.Context())) @@ -542,7 +542,7 @@ func TestChipIngressBatchEmitterService_PartialDeliveryError(t *testing.T) { cfg.ChipIngressMaxBatchSize = 1 cfg.ChipIngressSendInterval = time.Second - emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, newTestLogger(t)) + emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, logger.Test(t)) require.NoError(t, err) require.NoError(t, emitter.Start(t.Context())) @@ -589,7 +589,7 @@ func TestChipIngressBatchEmitterService_RPCError(t *testing.T) { cfg.ChipIngressMaxBatchSize = 1 cfg.ChipIngressSendInterval = time.Second - emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, newTestLogger(t)) + emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, logger.Test(t)) require.NoError(t, err) require.NoError(t, emitter.Start(t.Context())) @@ -620,7 +620,6 @@ func TestChipIngressBatchEmitterService_RPCError(t *testing.T) { }) } - func TestChipIngressBatchEmitterService_Metrics(t *testing.T) { t.Run("records events_sent on successful publish", func(t *testing.T) { reader, restore := useEmitterTestMeterProvider(t) @@ -638,7 +637,7 @@ func TestChipIngressBatchEmitterService_Metrics(t *testing.T) { cfg := newTestConfig() cfg.ChipIngressMaxBatchSize = 1 cfg.ChipIngressSendInterval = time.Second - emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, newTestLogger(t)) + emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, cfg, logger.Test(t)) require.NoError(t, err) require.NoError(t, emitter.Start(t.Context())) diff --git a/pkg/beholder/client_test.go b/pkg/beholder/client_test.go index 48dca5efff..bc3262f045 100644 --- a/pkg/beholder/client_test.go +++ b/pkg/beholder/client_test.go @@ -24,6 +24,7 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/chipingress" chipmocks "github.com/smartcontractkit/chainlink-common/pkg/chipingress/mocks" "github.com/smartcontractkit/chainlink-common/pkg/chipingress/pb" + "github.com/smartcontractkit/chainlink-common/pkg/logger" "github.com/smartcontractkit/chainlink-common/pkg/services" ) @@ -947,7 +948,7 @@ func TestClient_batchEmitterService(t *testing.T) { ChipIngressEmitterGRPCEndpoint: "localhost:9090", ChipIngressInsecureConnection: true, ChipIngressBatchEmitterEnabled: true, - ChipIngressLogger: newTestLogger(t), + ChipIngressLogger: logger.Test(t), ChipIngressBufferSize: 10, ChipIngressMaxBatchSize: 5, ChipIngressSendInterval: 50 * time.Millisecond, From 81f490927fd0d430c0f4bb0052c0c5b6f5c8377f Mon Sep 17 00:00:00 2001 From: Pavel <177363085+pkcll@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:02:00 -0400 Subject: [PATCH 2/2] fix: make gofix and check-gofix cover nested modules correctly - gofix now fans out across all go.mod files via gomods, so its "across all packages" description is accurate for this multi-module repo instead of only touching the root module. - check-gofix in CI only runs go fix in modules whose files actually changed, using dorny/paths-filter, to avoid unnecessary work on every push/PR. Addresses PR review comments: https://github.com/smartcontractkit/chainlink-common/pull/2280/changes#r3646569810 https://github.com/smartcontractkit/chainlink-common/pull/2280/changes#r3646569849 --- .github/workflows/pkg.yml | 51 +++++++++++++++++++++++++++++++++++++-- Makefile | 4 +-- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pkg.yml b/.github/workflows/pkg.yml index deca932e27..4e4ae8ef19 100644 --- a/.github/workflows/pkg.yml +++ b/.github/workflows/pkg.yml @@ -103,16 +103,63 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 + with: + persist-credentials: false + + - name: Detect changed modules + uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 + id: filter + with: + # "root" is the catch-all: any .go file outside the nested modules + # listed below, including locations not yet enumerated here (e.g. + # a new top-level package). + filters: | + root: + - '**/*.go' + - '!keystore/**' + - '!observability-lib/**' + - '!pkg/values/**' + - '!pkg/chipingress/**' + - '!pkg/monitoring/**' + - '!pkg/workflows/sdk/v2/pb/**' + keystore: + - 'keystore/**/*.go' + observability-lib: + - 'observability-lib/**/*.go' + pkg-values: + - 'pkg/values/**/*.go' + pkg-chipingress: + - 'pkg/chipingress/**/*.go' + pkg-monitoring: + - 'pkg/monitoring/**/*.go' + pkg-workflows-sdk-v2-pb: + - 'pkg/workflows/sdk/v2/pb/**/*.go' - name: Set up Go + if: steps.filter.outputs.root == 'true' || steps.filter.outputs.keystore == 'true' || steps.filter.outputs.observability-lib == 'true' || steps.filter.outputs.pkg-values == 'true' || steps.filter.outputs.pkg-chipingress == 'true' || steps.filter.outputs.pkg-monitoring == 'true' || steps.filter.outputs.pkg-workflows-sdk-v2-pb == 'true' uses: ./.github/actions/setup-go with: go-version-file: "go.mod" only-modules: "true" - - name: Ensure "make gofix" has been run + - name: Ensure "go fix" has been run on changed modules + if: steps.filter.outputs.root == 'true' || steps.filter.outputs.keystore == 'true' || steps.filter.outputs.observability-lib == 'true' || steps.filter.outputs.pkg-values == 'true' || steps.filter.outputs.pkg-chipingress == 'true' || steps.filter.outputs.pkg-monitoring == 'true' || steps.filter.outputs.pkg-workflows-sdk-v2-pb == 'true' + env: + ROOT_CHANGED: ${{ steps.filter.outputs.root }} + KEYSTORE_CHANGED: ${{ steps.filter.outputs.keystore }} + OBSERVABILITY_LIB_CHANGED: ${{ steps.filter.outputs.observability-lib }} + PKG_VALUES_CHANGED: ${{ steps.filter.outputs.pkg-values }} + PKG_CHIPINGRESS_CHANGED: ${{ steps.filter.outputs.pkg-chipingress }} + PKG_MONITORING_CHANGED: ${{ steps.filter.outputs.pkg-monitoring }} + PKG_WORKFLOWS_SDK_V2_PB_CHANGED: ${{ steps.filter.outputs.pkg-workflows-sdk-v2-pb }} run: | - make gofix + [[ "$ROOT_CHANGED" == "true" ]] && { (cd . && go fix ./...) || exit 1; } || true + [[ "$KEYSTORE_CHANGED" == "true" ]] && { (cd keystore && go fix ./...) || exit 1; } || true + [[ "$OBSERVABILITY_LIB_CHANGED" == "true" ]] && { (cd observability-lib && go fix ./...) || exit 1; } || true + [[ "$PKG_VALUES_CHANGED" == "true" ]] && { (cd pkg/values && go fix ./...) || exit 1; } || true + [[ "$PKG_CHIPINGRESS_CHANGED" == "true" ]] && { (cd pkg/chipingress && go fix ./...) || exit 1; } || true + [[ "$PKG_MONITORING_CHANGED" == "true" ]] && { (cd pkg/monitoring && go fix ./...) || exit 1; } || true + [[ "$PKG_WORKFLOWS_SDK_V2_PB_CHANGED" == "true" ]] && { (cd pkg/workflows/sdk/v2/pb && go fix ./...) || exit 1; } || true git add --all git diff --minimal --cached --exit-code diff --git a/Makefile b/Makefile index 62a53c3d52..d463f4266d 100644 --- a/Makefile +++ b/Makefile @@ -61,8 +61,8 @@ modgraph: gomods ./modgraph > go.md .PHONY: gofix -gofix: ## Run go fix across all packages - go fix ./... +gofix: gomods ## Run go fix across all packages + gomods -s proto_vendor -go fix ./... .PHONY: dependabot ifndef DEPENDABOT_SEVERITY