Skip to content

Conversation

@spikecurtis
Copy link
Collaborator

@spikecurtis spikecurtis commented Oct 21, 2025

fixes #22

Changes mock ticker and timer implementations based on channels to match Go 1.23+. c.f. https://go.dev/wiki/Go123Timer

  1. Channels for tickers and timers are now unbuffered.
  2. Channels reads will block after Stop() returns.
  3. We use goroutines to write to the unbuffered channels. In order to avoid leaking go routines, when the test ends, any uncompleted channel writes are abandoned. This means that reads from the channels will block after the test exits. This is usually what you want, but is a small difference in behavior from before, where we wrote to a buffered channel, and so reads could complete after the test completed.

@spikecurtis spikecurtis requested a review from mafredri October 21, 2025 11:01
@spikecurtis spikecurtis self-assigned this Oct 21, 2025
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, looks good for the most part. Had a suggestion for how to deal with the behavior difference, and one concern about cleanup.

@spikecurtis spikecurtis requested a review from mafredri October 30, 2025 07:33
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you whether you go with the proposal or not, if it even works (I may have overlooked something).

One last thing, I think it'd be good to call out the difference in behavior on public methods so it's visible in Go docs, potentially also in README if you find a good spot.

Copy link
Collaborator Author

spikecurtis commented Oct 30, 2025

Merge activity

  • Oct 30, 11:29 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Oct 30, 11:29 AM UTC: @spikecurtis merged this pull request with Graphite.

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.

Go 1.23 compatibility?

2 participants