From c4c790283b579a2c570ca30c629aaa75c51c4d11 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Fri, 1 May 2026 12:26:23 -0400 Subject: [PATCH 1/6] Test all modules --- .github/workflows/repo-hygiene.yaml | 4 +-- .github/workflows/test-coverage-report.yaml | 4 +-- .github/workflows/test-coverage.yaml | 2 +- Justfile | 29 +++++++++++++++------ 4 files changed, 26 insertions(+), 13 deletions(-) diff --git a/.github/workflows/repo-hygiene.yaml b/.github/workflows/repo-hygiene.yaml index bb9d35130..c1c5220b6 100644 --- a/.github/workflows/repo-hygiene.yaml +++ b/.github/workflows/repo-hygiene.yaml @@ -44,7 +44,7 @@ jobs: # Check that go.mod files have fully specificed Go versions that match tool-versions.env - name: Enforce go.mod Go versions - run: tools/bin/check-go-versions.sh + run: just check-go-versions - name: Run repo hygiene checks - run: tools/bin/check_repo_clean.sh tidy mock generate shellcheck + run: just check-repo-clean diff --git a/.github/workflows/test-coverage-report.yaml b/.github/workflows/test-coverage-report.yaml index 2b32090fd..582287cc3 100644 --- a/.github/workflows/test-coverage-report.yaml +++ b/.github/workflows/test-coverage-report.yaml @@ -47,14 +47,14 @@ jobs: - name: Run tests run: | - just test-coverage coverage.out short + just test-coverage coverage.out 1 - name: Coverage on target branch if: github.event_name == 'pull_request' run: | git fetch origin ${{ github.base_ref }} git branch -a git checkout ${{ github.base_ref }} - just test-coverage coverage_target.out short + just test-coverage coverage_target.out 1 # switch back to the head ref git checkout ${{ github.head_ref }} ./tools/bin/cov_compare.sh coverage_target.out coverage.out --label1="${{ github.base_ref }}" --label2="${{ github.head_ref }}" > table.txt diff --git a/.github/workflows/test-coverage.yaml b/.github/workflows/test-coverage.yaml index 1876200f9..687517a71 100644 --- a/.github/workflows/test-coverage.yaml +++ b/.github/workflows/test-coverage.yaml @@ -54,7 +54,7 @@ jobs: - name: Run tests run: | - just test-coverage coverage.out + just test-coverage total=$(go tool cover -func=coverage.out | grep total | awk '{print $3}') echo "Total coverage: $total" echo "$total" | awk -v threshold="$COVERAGE_THRESHOLD" '{ pc=substr($0, 1, length($0)-1); if (pc < threshold) { print "Coverage (" pc "%) is less than " threshold "%"; exit 1 } }' diff --git a/Justfile b/Justfile index e8e35e8d7..69b3a43ae 100644 --- a/Justfile +++ b/Justfile @@ -58,18 +58,31 @@ shellcheck: } find . -type f -name *.sh -execdir shellcheck {} + +check-go-versions: + ./tools/bin/check-go-versions.sh + +# Run hygiene targets and fail if any modify the repo. Default: tidy mock generate shellcheck +check-repo-clean +targets="tidy mock generate shellcheck": + ./tools/bin/check_repo_clean.sh {{targets}} + +# Render inter-module dependency graph as mermaid markdown +modgraph: ensure-go + ./tools/bin/modgraph.sh + mod-download: ensure-go go mod download -test short="": ensure-go - gomods -w go test -fullpath -shuffle on {{ if short != "" { "-short" } else { "" } }} -v -race ./... +test short="" all="": ensure-go + {{ if all != "" { "gomods -w" } else { "" } }} go test -fullpath -shuffle on {{ if short != "" { "-short" } else { "" } }} -v -race ./... + +# When all="1", generates / for each module (discoverable via find . -name coverage.out). +test-coverage coverage_file="coverage.out" short="" all="": + {{ if all != "" { "gomods -w" } else { "" } }} go test -v -race -fullpath -shuffle on {{ if short != "" { "-short" } else { "" } }} -coverprofile={{coverage_file}} ./... + {{ if all != "" { "gomods -w " } else { "" } }}sh -c '{ head -n1 {{coverage_file}}; tail -n +2 {{coverage_file}} | grep -v -E "{{COVERAGE_EXCLUDE_REGEX}}" || true; } > {{coverage_file}}.filtered && mv {{coverage_file}}.filtered {{coverage_file}}' -test-coverage coverage_file="coverage.out" short="": - # coverage_file := env_var_or_default('COVERAGE_FILE', 'coverage.out') - go test -v -race -fullpath -shuffle on {{ if short != "" { "-short" } else { "" } }} -v -coverprofile={{coverage_file}} ./... - # Filter mockery-generated files (mock_*.go) from coverage profile - { head -n1 {{coverage_file}}; tail -n +2 {{coverage_file}} | grep -v -E '{{COVERAGE_EXCLUDE_REGEX}}' || true; } > {{coverage_file}}.filtered - mv {{coverage_file}}.filtered {{coverage_file}} +# Compare per-module coverage between two profiles. Run test-coverage all=1 twice with different coverage_file names first. +cov-compare base="coverage_base.out" new="coverage.out": + gomods -c 'printf "\n### %s\n" "$(basename $PWD)"; {{justfile_directory()}}/tools/bin/cov_compare.sh "{{base}}" "{{new}}" || echo "(skipped — run: just test-coverage all=1)"' bump-chainlink-ccip sha: @echo "Bumping chainlink-ccip dependencies in root..." From 75a4e02fdf156ca3e5b1639dd1c4e844297686f6 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Fri, 1 May 2026 12:42:08 -0400 Subject: [PATCH 2/6] devenv fixes --- build/devenv/evm/event_poller.go | 7 ++++++- build/devenv/evm/event_poller_test.go | 5 ++--- build/devenv/tests/services/main_test.go | 5 +++++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/build/devenv/evm/event_poller.go b/build/devenv/evm/event_poller.go index c5d57d816..be593b1d4 100644 --- a/build/devenv/evm/event_poller.go +++ b/build/devenv/evm/event_poller.go @@ -165,9 +165,14 @@ func (p *eventPoller[T]) poll() { return } lastScanned := p.lastScannedBlock + client := p.ethClient p.mu.Unlock() - latestBlock, err := p.ethClient.BlockNumber(context.Background()) + if client == nil { + return + } + + latestBlock, err := client.BlockNumber(context.Background()) if err != nil { p.logger.Warn().Err(err).Str("event", p.eventName).Msg("Failed to get latest block number") return diff --git a/build/devenv/evm/event_poller_test.go b/build/devenv/evm/event_poller_test.go index ab420dc77..551c05f1e 100644 --- a/build/devenv/evm/event_poller_test.go +++ b/build/devenv/evm/event_poller_test.go @@ -171,9 +171,8 @@ func TestEventPollerByMessageID(t *testing.T) { messageID := protocol.Bytes32{3, 4, 5, 6} key := eventKey{chainSelector: 5, messageID: messageID} - ctx := context.Background() - ch1 := poller.registerByMessageID(ctx, key) - ch2 := poller.registerByMessageID(ctx, key) + ch1 := poller.registerByMessageID(t.Context(), key) + ch2 := poller.registerByMessageID(t.Context(), key) require.Equal(t, ch1, ch2, "registerByMessageID should return same channel for duplicate key registration") }) diff --git a/build/devenv/tests/services/main_test.go b/build/devenv/tests/services/main_test.go index 20baa7db9..a5246c645 100644 --- a/build/devenv/tests/services/main_test.go +++ b/build/devenv/tests/services/main_test.go @@ -1,6 +1,7 @@ package services_test import ( + "flag" "log" "os" "testing" @@ -9,6 +10,10 @@ import ( ) func TestMain(m *testing.M) { + flag.Parse() + if testing.Short() { + os.Exit(0) + } // to remove containers after the tests automatically _ = os.Setenv("TESTCONTAINERS_RYUK_DISABLED", "false") // to isolate containers the same way we do in e2e environment From bab0666e659f5148a550ebf2e93a535b0b392b94 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Fri, 1 May 2026 13:03:49 -0400 Subject: [PATCH 3/6] More things --- .../batch_write_commit_verifier_node_result.go | 9 +++++++-- .../pkg/handlers/write_commit_verifier_node_result.go | 10 ++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/aggregator/pkg/handlers/batch_write_commit_verifier_node_result.go b/aggregator/pkg/handlers/batch_write_commit_verifier_node_result.go index 802396e54..8027acbd4 100644 --- a/aggregator/pkg/handlers/batch_write_commit_verifier_node_result.go +++ b/aggregator/pkg/handlers/batch_write_commit_verifier_node_result.go @@ -58,10 +58,15 @@ func (h *BatchWriteCommitVerifierNodeResultHandler) Handle(ctx context.Context, if err != nil { statusErr, ok := grpcstatus.FromError(err) if !ok { - h.logger(ctx).Errorw("unexpected error type", "error", err) + if ctx.Err() == nil { + h.logger(ctx).Errorw("unexpected error type", "error", err) + } SetBatchError(errors, i, codes.Unknown, "internal error") } else { - h.logger(ctx).Errorw("failed to write commit verification node result", "error", statusErr) + code := statusErr.Code() + if code != codes.Canceled && code != codes.DeadlineExceeded { + h.logger(ctx).Errorw("failed to write commit verification node result", "error", statusErr) + } errors[i] = statusErr.Proto() } } else { diff --git a/aggregator/pkg/handlers/write_commit_verifier_node_result.go b/aggregator/pkg/handlers/write_commit_verifier_node_result.go index 465b72d65..09464eb6c 100644 --- a/aggregator/pkg/handlers/write_commit_verifier_node_result.go +++ b/aggregator/pkg/handlers/write_commit_verifier_node_result.go @@ -126,6 +126,16 @@ func (h *WriteCommitVerifierNodeResultHandler) Handle(ctx context.Context, req * }, status.Error(codes.ResourceExhausted, "service temporarily unavailable: aggregation queue full") } + if ctx.Err() != nil { + code := codes.DeadlineExceeded + if ctx.Err() == context.Canceled { + code = codes.Canceled + } + return &committeepb.WriteCommitteeVerifierNodeResultResponse{ + Status: committeepb.WriteStatus_FAILED, + }, status.Error(code, "request cancelled") + } + reqLogger.Errorw("failed to trigger aggregation", "error", err) return &committeepb.WriteCommitteeVerifierNodeResultResponse{ Status: committeepb.WriteStatus_FAILED, From b27524a5f89dfde5eb98d68efa6dc7c217d84fa8 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Fri, 1 May 2026 13:11:30 -0400 Subject: [PATCH 4/6] Kill the integ tests in short mode. --- .../tests/composable/messaging/main_test.go | 15 +++++++++++++++ build/devenv/tests/e2e/main_test.go | 15 +++++++++++++++ build/devenv/tests/services/load/main_test.go | 15 +++++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 build/devenv/tests/composable/messaging/main_test.go create mode 100644 build/devenv/tests/e2e/main_test.go create mode 100644 build/devenv/tests/services/load/main_test.go diff --git a/build/devenv/tests/composable/messaging/main_test.go b/build/devenv/tests/composable/messaging/main_test.go new file mode 100644 index 000000000..7e72f7602 --- /dev/null +++ b/build/devenv/tests/composable/messaging/main_test.go @@ -0,0 +1,15 @@ +package messaging + +import ( + "flag" + "os" + "testing" +) + +func TestMain(m *testing.M) { + flag.Parse() + if testing.Short() { + os.Exit(0) + } + os.Exit(m.Run()) +} diff --git a/build/devenv/tests/e2e/main_test.go b/build/devenv/tests/e2e/main_test.go new file mode 100644 index 000000000..0c57ba456 --- /dev/null +++ b/build/devenv/tests/e2e/main_test.go @@ -0,0 +1,15 @@ +package e2e + +import ( + "flag" + "os" + "testing" +) + +func TestMain(m *testing.M) { + flag.Parse() + if testing.Short() { + os.Exit(0) + } + os.Exit(m.Run()) +} diff --git a/build/devenv/tests/services/load/main_test.go b/build/devenv/tests/services/load/main_test.go new file mode 100644 index 000000000..353778da3 --- /dev/null +++ b/build/devenv/tests/services/load/main_test.go @@ -0,0 +1,15 @@ +package load + +import ( + "flag" + "os" + "testing" +) + +func TestMain(m *testing.M) { + flag.Parse() + if testing.Short() { + os.Exit(0) + } + os.Exit(m.Run()) +} From 2d0359a5abd16eb03f7dc7dbd634ecc3d6b8af43 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Fri, 1 May 2026 14:36:03 -0400 Subject: [PATCH 5/6] lint fix --- ...batch_write_commit_verifier_node_result.go | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/aggregator/pkg/handlers/batch_write_commit_verifier_node_result.go b/aggregator/pkg/handlers/batch_write_commit_verifier_node_result.go index 8027acbd4..e89866e4c 100644 --- a/aggregator/pkg/handlers/batch_write_commit_verifier_node_result.go +++ b/aggregator/pkg/handlers/batch_write_commit_verifier_node_result.go @@ -5,13 +5,13 @@ import ( "fmt" "sync" + rpcstatus "google.golang.org/genproto/googleapis/rpc/status" "google.golang.org/grpc/codes" + grpcstatus "google.golang.org/grpc/status" "github.com/smartcontractkit/chainlink-ccv/aggregator/pkg/scope" "github.com/smartcontractkit/chainlink-common/pkg/logger" - grpcstatus "google.golang.org/grpc/status" - committeepb "github.com/smartcontractkit/chainlink-protos/chainlink-ccv/committee-verifier/v1" ) @@ -25,6 +25,22 @@ func (h *BatchWriteCommitVerifierNodeResultHandler) logger(ctx context.Context) return scope.AugmentLogger(ctx, h.handler.l) } +func (h *BatchWriteCommitVerifierNodeResultHandler) handleBatchItemError(ctx context.Context, errors []*rpcstatus.Status, i int, err error) { + statusErr, ok := grpcstatus.FromError(err) + if !ok { + if ctx.Err() == nil { + h.logger(ctx).Errorw("unexpected error type", "error", err) + } + SetBatchError(errors, i, codes.Unknown, "internal error") + return + } + code := statusErr.Code() + if code != codes.Canceled && code != codes.DeadlineExceeded { + h.logger(ctx).Errorw("failed to write commit verification node result", "error", statusErr) + } + errors[i] = statusErr.Proto() +} + // Handle processes the write request and saves the commit verification record. // The parent context includes a timeout from RequestTimeoutMiddleware to prevent goroutine leaks. func (h *BatchWriteCommitVerifierNodeResultHandler) Handle(ctx context.Context, req *committeepb.BatchWriteCommitteeVerifierNodeResultRequest) (*committeepb.BatchWriteCommitteeVerifierNodeResultResponse, error) { @@ -55,23 +71,12 @@ func (h *BatchWriteCommitVerifierNodeResultHandler) Handle(ctx context.Context, return } resp, err := h.handler.Handle(ctx, r) - if err != nil { - statusErr, ok := grpcstatus.FromError(err) - if !ok { - if ctx.Err() == nil { - h.logger(ctx).Errorw("unexpected error type", "error", err) - } - SetBatchError(errors, i, codes.Unknown, "internal error") - } else { - code := statusErr.Code() - if code != codes.Canceled && code != codes.DeadlineExceeded { - h.logger(ctx).Errorw("failed to write commit verification node result", "error", statusErr) - } - errors[i] = statusErr.Proto() - } - } else { + if err == nil { SetBatchSuccess(errors, i) + responses[i] = resp + return } + h.handleBatchItemError(ctx, errors, i, err) responses[i] = resp }(i, r) } From 074eda0a43cb3eefe63d1ca3bc98688732f22cd7 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Fri, 1 May 2026 14:56:52 -0400 Subject: [PATCH 6/6] janky fix because these changes aren't in the target branch --- .github/workflows/test-coverage-report.yaml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-coverage-report.yaml b/.github/workflows/test-coverage-report.yaml index 582287cc3..d3afc48a6 100644 --- a/.github/workflows/test-coverage-report.yaml +++ b/.github/workflows/test-coverage-report.yaml @@ -54,10 +54,12 @@ jobs: git fetch origin ${{ github.base_ref }} git branch -a git checkout ${{ github.base_ref }} - just test-coverage coverage_target.out 1 + # Allow test failures on the target branch — a flaky test there should not block the PR. + # Coverage data is still written for all packages that did not panic. + just test-coverage coverage_target.out 1 || echo "Warning: some tests failed on target branch; coverage may be incomplete" # switch back to the head ref git checkout ${{ github.head_ref }} - ./tools/bin/cov_compare.sh coverage_target.out coverage.out --label1="${{ github.base_ref }}" --label2="${{ github.head_ref }}" > table.txt + ./tools/bin/cov_compare.sh coverage_target.out coverage.out --label1="${{ github.base_ref }}" --label2="${{ github.head_ref }}" > table.txt || echo "(coverage comparison unavailable)" > table.txt cat table.txt { echo 'coverage_report<