Upgrade internal/tools go module for x/tools and mockery #1421

Groxx · 2025-05-05T18:38:16Z

Mockery is failing when run with go 1.24 due to vektra/mockery#914 , so let's upgrade it.

Basic steps to upgrade are:

# add go toolchain to go.mod to enforce a higher minimum
cd internal/tools
go get golang.org/x/tools@latest github.com/vektra/mockery/v2@latest
cd ../..
make all

and some careful reading to verify the results.

And then some changes to address our CI needs, e.g. since runtime and tool-time versions have diverged (currently) we now automatically pull and build with the old version.
Most of these version values can be found by doing a grep like

❯ rg 'go(lang)?.?1.?2\d'
go.mod
3:go 1.21                # intentionally different

docker/buildkite/Dockerfile
1:FROM golang:1.24

Makefile
36:EXPECTED_GO_VERSION := go1.24

CHANGELOG.md
28:- Bump x/tools for tools, to support go 1.22 (#1336)
143:- Fix TestActivityWorkerStop: it times out with go 1.20 by @dkrotx in #1223

internal/tools/go.mod
3:go 1.23.0
5:toolchain go1.24.2

I had expected go 1.21 in go.mod to take care of this kind of check, but apparently not: golang/go#73603
so it's now done by hand.

There are some changes which I think stand a very small chance of causing problems:

Our RPC client's mocks now check for .Get(0).(func(args) (any, error) signatures where before they did not
- I believe anyone using this would've hit a .Error further down after these changes, so hopefully this just fixes buggy behavior and does not cause any existing tests to fail
Many funcs gained a if len(ret) == 0 { panic(..) } check
- Since there's a ret.Get(0) immediately after which also panics, I suspect this just gives better error messages.
Our mocks now have a DoAndReturn method
- Since there are no interfaces for all this, this should be fine, ignoring reflection (which we must ignore or else we will go mad).

Otherwise seems like nothing exciting, and this does not affect any user-facing libraries or Go versions.
Since this needs a newer CI Go version to build, I've updated those references too.

Separately, this was the source of some rather disturbing TILs:

❯ bash -ec '
[[[ "$a" = "foo"]] && echo first
echo second
'
bash: line 2: [[[: command not found
second

❯ echo $?
0

Apparently syntax errors are (sometimes) not script failures, even with set -e.

And sh does not support [[, so it was printing an error and continuing without erroring. CI uses sh, locally we generally get bash or zsh, so a locally-working [[ may be a remote-silently-failing command.

Madness. Absolute madness. This is going to haunt me for a while, unless I can find a way to prevent it (without shellcheck).

Mockery is failing when run with go 1.24 due to vektra/mockery#914 , so let's upgrade it. Steps to create this diff are just: ``` cd internal/tools go get golang.org/x/tools@latest github.com/vektra/mockery/v2@latest cd ../.. make all ``` and some careful reading to verify the results. There are *some* changes which I think stand a small chance of causing problems: - Our RPC client's mocks now check for `.Get(0).(func(args) (any, error)` signatures where before they did not - I believe anyone using this would've hit a `.Error` further down after these changes, so hopefully this just fixes buggy behavior and does not cause any existing tests to fail - Many funcs gained a `if len(ret) == 0 { panic(..) }` check - Since there's a `ret.Get(0)` immediately after which also panics, I suspect this just gives better error messages. - Our mocks now have a `DoAndReturn` method - Since there are no interfaces for all this, this should be fine, ignoring reflection (which we must ignore or else we will go mad). Otherwise seems like nothing exciting, and this does not affect any user-facing libraries or Go versions. Since this needs a newer CI Go version to build, I've updated those references too.

codecov · 2025-05-05T20:42:13Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.70%. Comparing base (d9dcddc) to head (815d0c2).
Report is 1 commits behind head on master.

see 1 file with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9dcddc...815d0c2. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Groxx · 2025-05-05T20:51:13Z

Makefile

+	$Q # compare the current go version with what is in go.mod, and get `{gomod_version}.1` or similar and ensure it builds.
+	$Q GOVERSION="$(shell go env GOVERSION | cut -d. -f1-2)" \
+		MODVERSION="go$(shell grep '^go 1' go.mod | cut -d' ' -f2)"; \
+		if [ "$$GOVERSION" != "$$MODVERSION" ]; then \


so this was the source of some rather disturbing TILs:

❯ bash -ec ' [[[ "$a" = "foo"]] && echo first echo second ' bash: line 2: [[[: command not found second ❯ echo $? 0

Apparently syntax errors are (sometimes) not script failures, even with set -e.

And sh does not support [[, so it was printing an error and continuing without erroring. CI uses sh, locally we generally get bash or zsh.

Madness. Absolute madness. This is going to haunt me for a while, unless I can find a way to prevent it (without shellcheck).

Groxx · 2025-05-05T20:51:37Z

internal/internal_workflow_task_handler_mock.go

+	if len(ret) == 0 {
+		panic("no return value specified for ProcessWorkflowTask")
+	}


.Get(0) would panic, so this should just be a better error message

Groxx · 2025-05-05T20:52:36Z

internal/internal_workflow_task_handler_mock.go

+	if rf, ok := ret.Get(0).(func(*workflowTask, decisionHeartbeatFunc) (interface{}, error)); ok {
+		return rf(task, f)
+	}


Below there is this:

if ret.Get(0) != nil { r0 = ret.Get(0).(interface{}) }

which should have panicked if the new signature was used, so I believe this is a safe addition / nobody could have been passing this value in previously.

Groxx · 2025-05-05T20:53:00Z

internal/internal_workflow_task_handler_mock.go

-type mockConstructorTestingTNewMockWorkflowTaskHandler interface {
-	mock.TestingT
-	Cleanup(func())
+func (_c *MockWorkflowTaskHandler_ProcessWorkflowTask_Call) RunAndReturn(run func(*workflowTask, decisionHeartbeatFunc) (interface{}, error)) *MockWorkflowTaskHandler_ProcessWorkflowTask_Call {


new mock method, should be fine as it's not on an interface

Groxx requested review from shijiesheng, jakobht, 3vilhamster, dkrotx, taylanisikdemir and demirkayaender as code owners May 5, 2025 18:38

update changelog

1d0624a

shijiesheng mentioned this pull request May 5, 2025

upgrade mockery version for go 1.24 support #1420

Closed

shijiesheng approved these changes May 5, 2025

View reviewed changes

taylanisikdemir approved these changes May 5, 2025

View reviewed changes

Groxx added 10 commits May 5, 2025 14:11

force a check for go 1.21, speed up validate as it does not need tests

9193495

misunderstood that compose instruction, undoing

98437c7

forcing a go.mod-based build check

0596f6f

remove extra build step

d2cf457

update codecov version

de93533

ci uses /bin/sh not /bin/bash

cd25163

add missing -e

db8af5c

re-break /bin/sh temporarilyg

1a391db

try harder to break things

4609957

fix syntax for /bin/sh

d0ea7d5

Groxx added 2 commits May 5, 2025 15:45

break 1.21 temporarily=

d933d53

fixed, verified

815d0c2

Groxx commented May 5, 2025

View reviewed changes

Groxx merged commit dd7df58 into cadence-workflow:master May 5, 2025
10 checks passed

Groxx deleted the mockery-upgrade branch May 5, 2025 21:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Upgrade internal/tools go module for x/tools and mockery #1421

Upgrade internal/tools go module for x/tools and mockery #1421

Uh oh!

Groxx commented May 5, 2025 •

edited

Loading

Uh oh!

codecov bot commented May 5, 2025 •

edited

Loading

Uh oh!

Groxx May 5, 2025

Uh oh!

Groxx May 5, 2025

Uh oh!

Groxx May 5, 2025

Uh oh!

Groxx May 5, 2025

Uh oh!

Uh oh!

Uh oh!

Upgrade internal/tools go module for x/tools and mockery #1421

Upgrade internal/tools go module for x/tools and mockery #1421

Uh oh!

Conversation

Groxx commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Groxx May 5, 2025

Choose a reason for hiding this comment

Uh oh!

Groxx May 5, 2025

Choose a reason for hiding this comment

Uh oh!

Groxx May 5, 2025

Choose a reason for hiding this comment

Uh oh!

Groxx May 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Groxx commented May 5, 2025 •

edited

Loading

codecov bot commented May 5, 2025 •

edited

Loading