Skip to content

🔨 Upgrade to Go 1.25#50

Merged
kulti merged 9 commits intokulti:masterfrom
busser:arthur/go-1.25
Sep 1, 2025
Merged

🔨 Upgrade to Go 1.25#50
kulti merged 9 commits intokulti:masterfrom
busser:arthur/go-1.25

Conversation

@busser
Copy link
Copy Markdown
Contributor

@busser busser commented Aug 27, 2025

We want thelper to include a built-in exception for synctest.Test
(see #48). This requires using Go 1.25, where the testing/synctest
package became generally available. However, tests fail with newer
versions of Go because we're using a very old version of
golang.org/x/tools which is incompatbile with newer versions of Go
(see golang/go#62167).

This PR upgrades thelper to Go 1.25 and the latest version of
golang.org/x/tools. Tests now pass.

We'll add support for synctest.Test in a follow-up PR.

We want `thelper` to include a built-in exception for `synctest.Test`
(see kulti#48). This requires using Go 1.25, where the `testing/synctest`
package became generally available. However, tests fail with newer
versions of Go because we're using a very old version of
`golang.org/x/tools` which is incompatbile with newer versions of Go
(see golang/go#62167).

This PR upgrades `thelper` to Go 1.25 and the latest version of
`golang.org/x/tools`. Tests now pass.

We'll add support for `synctest.Test` in a follow-up PR.
@ccoVeille
Copy link
Copy Markdown

golang.org/x/tools 0.1.10 is indeed very old.

Maybe it's time to consider using dependabot

@busser
Copy link
Copy Markdown
Contributor Author

busser commented Aug 27, 2025

Maybe it's time to consider using dependabot

I've had good experiences with Renovate (by Mend). Very easy to set up and reliably opens PRs for the go runtime/compiler and go packages.

Feel free to use what you prefer, of course.

@ccoVeille
Copy link
Copy Markdown

I'm a random Gopher commenting your PR.

I will let maintainers to provide feedbacks.

I'm fine with dependabot or renovate. The important is the feature of updating things

busser added a commit to busser/thelper that referenced this pull request Aug 27, 2025
Go 1.25 made the `testing/synctest` package generally available. The
package introduces the `synctest.Test` function, to be used like this:

```go
func TestMyFeature(t *testing.T) {
	synctest.Test(t, func(t *testing.T) {
		// Do stuff that uses goroutines
	})
}
```

Sadly, `thelper` triggers if the anonymous function passed to
`synctest.Test` doesn't call `t.Helper()`. The issue is that this
anonymous function isn't a helper, it's more like a sub-test.

This PR extends `thelper` to grant an exception to functions passed to
`synctest.Test`, just like functions passed to `t.Run` or `b.Run`.

This requires updating to Go 1.25, so this PR is stacked on top of kulti#50.
@busser
Copy link
Copy Markdown
Contributor Author

busser commented Aug 28, 2025

CI failed because it was installing Go 1.18 rather than Go 1.25. There's a known issue (golang/go#75031) where go test -cover fails on packages without tests when an older version of Go downloads the 1.25 toolchain on the fly. It's a weird bug. It's easily worked around by downloading Go 1.25 directly. I've updated the CI files accordingly.

@busser
Copy link
Copy Markdown
Contributor Author

busser commented Aug 28, 2025

I made a mistake: I configured setup-go to detect the Go version we need by reading go.mod. That failed because the jobs clone the repository after installing Go. The fix is simple: clone the repository first.

@kulti
Copy link
Copy Markdown
Owner

kulti commented Aug 28, 2025

Well, something other is broken in golangci-lint configuration and coveralls. I can sort this out on the next week.

Thank you @busser for Go version updating and improving wokrkflows!

@busser
Copy link
Copy Markdown
Contributor Author

busser commented Aug 29, 2025

My pleasure @kulti. I like thelper, thanks for making it! We use it where I work. We now use synctest and want to keep using thelper. I'm happy to help on this!

@busser
Copy link
Copy Markdown
Contributor Author

busser commented Sep 1, 2025

Hey @kulti, I got the code to compile, tests to pass, and linter checks to pass too 🎉

There's a code coverage job that's still failing, but I'm not really clear on why. The PR doesn't contain any significant code changes that would affect code coverage so significantly. I added a bit of whitespace to satisfy the linters, but that shouldn't affect code coverage 🤔

Any thoughts on what we should do?

I would consider making the coverage job non-mandatory, but that's your call.

@kulti
Copy link
Copy Markdown
Owner

kulti commented Sep 1, 2025

I suppose coveralls changes rules to detect relevant lines. I see, that it was 441 before changes and should be 451, but coveralls says that is 520 relevant lines. So percentage is dramatically decreased.

It's not a big deal. Merged 🚀 Thanks for your contribution ❤️

@kulti kulti merged commit d9df2e9 into kulti:master Sep 1, 2025
3 of 4 checks passed
busser added a commit to busser/thelper that referenced this pull request Sep 2, 2025
Go 1.25 made the `testing/synctest` package generally available. The
package introduces the `synctest.Test` function, to be used like this:

```go
func TestMyFeature(t *testing.T) {
	synctest.Test(t, func(t *testing.T) {
		// Do stuff that uses goroutines
	})
}
```

Sadly, `thelper` triggers if the anonymous function passed to
`synctest.Test` doesn't call `t.Helper()`. The issue is that this
anonymous function isn't a helper, it's more like a sub-test.

This PR extends `thelper` to grant an exception to functions passed to
`synctest.Test`, just like functions passed to `t.Run` or `b.Run`.

This requires updating to Go 1.25, so this PR is stacked on top of kulti#50.
busser added a commit to busser/thelper that referenced this pull request Sep 2, 2025
Go 1.25 made the `testing/synctest` package generally available. The
package introduces the `synctest.Test` function, to be used like this:

```go
func TestMyFeature(t *testing.T) {
	synctest.Test(t, func(t *testing.T) {
		// Do stuff that uses goroutines
	})
}
```

Sadly, `thelper` triggers if the anonymous function passed to
`synctest.Test` doesn't call `t.Helper()`. The issue is that this
anonymous function isn't a helper, it's more like a sub-test.

This PR extends `thelper` to grant an exception to functions passed to
`synctest.Test`, just like functions passed to `t.Run` or `b.Run`.

This requires updating to Go 1.25, so this PR is stacked on top of kulti#50.
kulti pushed a commit that referenced this pull request Sep 15, 2025
Go 1.25 made the `testing/synctest` package generally available. The
package introduces the `synctest.Test` function, to be used like this:

```go
func TestMyFeature(t *testing.T) {
	synctest.Test(t, func(t *testing.T) {
		// Do stuff that uses goroutines
	})
}
```

Sadly, `thelper` triggers if the anonymous function passed to
`synctest.Test` doesn't call `t.Helper()`. The issue is that this
anonymous function isn't a helper, it's more like a sub-test.

This PR extends `thelper` to grant an exception to functions passed to
`synctest.Test`, just like functions passed to `t.Run` or `b.Run`.

This requires updating to Go 1.25, so this PR is stacked on top of #50.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants