From 491b6981f705888982f8a76c8a0f174fd25f7569 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Wed, 7 Feb 2024 04:59:31 -0800 Subject: [PATCH 1/7] test: add a custom mock clock implementation benbjohnson/clock has been archived and no longer maintained. Removing that dependency is desirable. This change adds a mock clock adapted from Zap's mock clock (uber-go/zap#1349), but with operations specific to Fx's needs. There are two areas that require more scrutiny because of divergence from Zap's mock clock: WithTimeout requires us to implement a custom `context.Context` so that it can report context.DeadlineExceeded when it's time. We cannot just use `context.WithCancelCause` here because the cause for a context failure is considered different from `ctx.Err`, so `ctx.Err` would still report `context.Canceled` for a timeout. When testing that a sleep behaves as expected, there's a data race between the `Sleep` and the `Add` that progresses time. To resolve this, this change adds an `AwaitScheduled` method that blocks until there are operations scheduled for the future. This is done by using a `sync.Cond`. This gives us a way to wait until the sleep is scheduled before we progress time. This change also had to update Zap to pick up the release with the custom clock to drop the benbjohnson/clock dependency from the go.mod completely. Refs uber-go/zap#1349 Resolves #1135 --- app_test.go | 22 ++-- docs/go.mod | 7 +- docs/go.sum | 18 ++- go.mod | 9 +- go.sum | 52 ++------- internal/e2e/go.mod | 5 +- internal/e2e/go.sum | 14 +-- internal/fxclock/clock.go | 204 ++++++++++++++++++++++++++++++++- internal/fxclock/clock_test.go | 146 +++++++++++++++++++---- 9 files changed, 369 insertions(+), 108 deletions(-) diff --git a/app_test.go b/app_test.go index c9b9000b0..a45b4cd37 100644 --- a/app_test.go +++ b/app_test.go @@ -35,13 +35,13 @@ import ( "testing" "time" - "github.com/benbjohnson/clock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" . "go.uber.org/fx" "go.uber.org/fx/fxevent" "go.uber.org/fx/fxtest" + "go.uber.org/fx/internal/fxclock" "go.uber.org/fx/internal/fxlog" "go.uber.org/goleak" "go.uber.org/multierr" @@ -1221,7 +1221,7 @@ func TestAppRunTimeout(t *testing.T) { } // Builds a hook that takes much longer than the application timeout. - takeVeryLong := func(clock *clock.Mock) func() error { + takeVeryLong := func(clock *fxclock.Mock) func() error { return func() error { // We'll exceed the start and stop timeouts, // and then some. @@ -1237,7 +1237,7 @@ func TestAppRunTimeout(t *testing.T) { desc string // buildHook builds and returns the hooks for this test case. - buildHooks func(*clock.Mock) []Hook + buildHooks func(*fxclock.Mock) []Hook // Type of the fxevent we want. // Does not reflect the exact value. @@ -1246,7 +1246,7 @@ func TestAppRunTimeout(t *testing.T) { { // Timeout starting an application. desc: "OnStart timeout", - buildHooks: func(clock *clock.Mock) []Hook { + buildHooks: func(clock *fxclock.Mock) []Hook { return []Hook{ StartHook(takeVeryLong(clock)), } @@ -1256,7 +1256,7 @@ func TestAppRunTimeout(t *testing.T) { { // Timeout during a rollback because start failed. desc: "rollback timeout", - buildHooks: func(clock *clock.Mock) []Hook { + buildHooks: func(clock *fxclock.Mock) []Hook { return []Hook{ // The hooks are separate because // OnStop will not be run if that hook failed. @@ -1269,7 +1269,7 @@ func TestAppRunTimeout(t *testing.T) { { // Timeout during a stop. desc: "OnStop timeout", - buildHooks: func(clock *clock.Mock) []Hook { + buildHooks: func(clock *fxclock.Mock) []Hook { return []Hook{ StopHook(takeVeryLong(clock)), } @@ -1283,7 +1283,7 @@ func TestAppRunTimeout(t *testing.T) { t.Run(tt.desc, func(t *testing.T) { t.Parallel() - mockClock := clock.NewMock() + mockClock := fxclock.NewMock() var ( exitCode int @@ -1351,7 +1351,7 @@ func TestAppStart(t *testing.T) { t.Run("Timeout", func(t *testing.T) { t.Parallel() - mockClock := clock.NewMock() + mockClock := fxclock.NewMock() type A struct{} blocker := func(lc Lifecycle) *A { @@ -1388,7 +1388,7 @@ func TestAppStart(t *testing.T) { t.Run("TimeoutWithFinishedHooks", func(t *testing.T) { t.Parallel() - mockClock := clock.NewMock() + mockClock := fxclock.NewMock() type A struct{} type B struct{ A *A } @@ -1540,7 +1540,7 @@ func TestAppStart(t *testing.T) { t.Parallel() var ran bool - mockClock := clock.NewMock() + mockClock := fxclock.NewMock() app := New( WithClock(mockClock), Invoke(func(lc Lifecycle) { @@ -1820,7 +1820,7 @@ func TestAppStop(t *testing.T) { t.Run("Timeout", func(t *testing.T) { t.Parallel() - mockClock := clock.NewMock() + mockClock := fxclock.NewMock() block := func(ctx context.Context) error { mockClock.Add(5 * time.Second) diff --git a/docs/go.mod b/docs/go.mod index 865b30b7b..1f4d20db5 100644 --- a/docs/go.mod +++ b/docs/go.mod @@ -3,17 +3,16 @@ module go.uber.org/fx/docs go 1.20 require ( - github.com/stretchr/testify v1.8.0 + github.com/stretchr/testify v1.8.1 go.uber.org/fx v1.18.2 - go.uber.org/zap v1.23.0 + go.uber.org/zap v1.26.0 ) require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - go.uber.org/atomic v1.7.0 // indirect go.uber.org/dig v1.17.1 // indirect - go.uber.org/multierr v1.6.0 // indirect + go.uber.org/multierr v1.10.0 // indirect golang.org/x/sys v0.0.0-20220412211240-33da011f77ad // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/docs/go.sum b/docs/go.sum index 83453e186..2e829754b 100644 --- a/docs/go.sum +++ b/docs/go.sum @@ -1,25 +1,23 @@ -github.com/benbjohnson/clock v1.3.0 h1:ip6w0uFQkncKQ979AypyG0ER7mqUSBdKLOgAle/AT8A= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= -github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= +github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw= -go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/dig v1.17.1 h1:Tga8Lz8PcYNsWsyHMZ1Vm0OQOUaJNDyvPImgbAu9YSc= go.uber.org/dig v1.17.1/go.mod h1:Us0rSJiThwCv2GteUN0Q7OKvU7n5J4dxZ9JKUXozFdE= -go.uber.org/goleak v1.1.11 h1:wy28qYRKZgnJTxGxvye5/wgWr1EKjmUDGYox5mGlRlI= -go.uber.org/multierr v1.6.0 h1:y6IPFStTAIT5Ytl7/XYmHvzXQ7S3g/IeZW9hyZ5thw4= -go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= -go.uber.org/zap v1.23.0 h1:OjGQ5KQDEUawVHxNwQgPpiypGHOxo2mNZsOqTak4fFY= -go.uber.org/zap v1.23.0/go.mod h1:D+nX8jyLsMHMYrln8A0rJjFt/T/9/bGgIhAqxv5URuY= +go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk= +go.uber.org/multierr v1.10.0 h1:S0h4aNzvfcFsC3dRF1jLoaov7oRaKqRGC/pUEJ2yvPQ= +go.uber.org/multierr v1.10.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= +go.uber.org/zap v1.26.0 h1:sI7k6L95XOKS281NhVKOFCUNIvv9e0w4BF8N3u+tCRo= +go.uber.org/zap v1.26.0/go.mod h1:dtElttAiwGvoJ/vj4IwHBS/gXsEu/pZ50mUIRWuG0so= golang.org/x/sys v0.0.0-20220412211240-33da011f77ad h1:ntjMns5wyP/fN65tdBD4g8J5w8n015+iIIs9rtjXkY0= golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= diff --git a/go.mod b/go.mod index 777417de1..e322f63d5 100644 --- a/go.mod +++ b/go.mod @@ -3,13 +3,12 @@ module go.uber.org/fx go 1.20 require ( - github.com/benbjohnson/clock v1.3.0 - github.com/stretchr/testify v1.8.0 + github.com/stretchr/testify v1.8.1 go.uber.org/atomic v1.7.0 go.uber.org/dig v1.17.1 - go.uber.org/goleak v1.1.11 - go.uber.org/multierr v1.6.0 - go.uber.org/zap v1.23.0 + go.uber.org/goleak v1.2.0 + go.uber.org/multierr v1.10.0 + go.uber.org/zap v1.26.0 golang.org/x/sys v0.0.0-20220412211240-33da011f77ad ) diff --git a/go.sum b/go.sum index 443ecde43..b20efccb7 100644 --- a/go.sum +++ b/go.sum @@ -1,66 +1,34 @@ -github.com/benbjohnson/clock v1.3.0 h1:ip6w0uFQkncKQ979AypyG0ER7mqUSBdKLOgAle/AT8A= -github.com/benbjohnson/clock v1.3.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= -github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= -github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= -github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= -github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= -github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= -github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= -github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= +github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= +github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw= go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/dig v1.17.1 h1:Tga8Lz8PcYNsWsyHMZ1Vm0OQOUaJNDyvPImgbAu9YSc= go.uber.org/dig v1.17.1/go.mod h1:Us0rSJiThwCv2GteUN0Q7OKvU7n5J4dxZ9JKUXozFdE= -go.uber.org/goleak v1.1.11 h1:wy28qYRKZgnJTxGxvye5/wgWr1EKjmUDGYox5mGlRlI= -go.uber.org/goleak v1.1.11/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ= -go.uber.org/multierr v1.6.0 h1:y6IPFStTAIT5Ytl7/XYmHvzXQ7S3g/IeZW9hyZ5thw4= -go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= -go.uber.org/zap v1.23.0 h1:OjGQ5KQDEUawVHxNwQgPpiypGHOxo2mNZsOqTak4fFY= -go.uber.org/zap v1.23.0/go.mod h1:D+nX8jyLsMHMYrln8A0rJjFt/T/9/bGgIhAqxv5URuY= -golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk= +go.uber.org/goleak v1.2.0/go.mod h1:XJYK+MuIchqpmGmUSAzotztawfKvYLUIgg7guXrwVUo= +go.uber.org/multierr v1.10.0 h1:S0h4aNzvfcFsC3dRF1jLoaov7oRaKqRGC/pUEJ2yvPQ= +go.uber.org/multierr v1.10.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= +go.uber.org/zap v1.26.0 h1:sI7k6L95XOKS281NhVKOFCUNIvv9e0w4BF8N3u+tCRo= +go.uber.org/zap v1.26.0/go.mod h1:dtElttAiwGvoJ/vj4IwHBS/gXsEu/pZ50mUIRWuG0so= golang.org/x/lint v0.0.0-20190930215403-16217165b5de h1:5hukYrvBGR8/eNkX5mdUezrA6JiaEZDtJb9Ei+1LlBs= -golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= -golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= -golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= -golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= -golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220412211240-33da011f77ad h1:ntjMns5wyP/fN65tdBD4g8J5w8n015+iIIs9rtjXkY0= golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= -golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= -golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.5 h1:ouewzE6p+/VEB31YYnTbEJdi8pFqKp4P4n85vwo3DHA= -golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= -golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= -gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/e2e/go.mod b/internal/e2e/go.mod index 5335cc330..3f0ed8f99 100644 --- a/internal/e2e/go.mod +++ b/internal/e2e/go.mod @@ -10,10 +10,9 @@ require ( require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - go.uber.org/atomic v1.7.0 // indirect go.uber.org/dig v1.17.1 // indirect - go.uber.org/multierr v1.6.0 // indirect - go.uber.org/zap v1.23.0 // indirect + go.uber.org/multierr v1.10.0 // indirect + go.uber.org/zap v1.26.0 // indirect golang.org/x/sys v0.0.0-20220412211240-33da011f77ad // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/internal/e2e/go.sum b/internal/e2e/go.sum index 36c0e3960..e5abd8557 100644 --- a/internal/e2e/go.sum +++ b/internal/e2e/go.sum @@ -1,27 +1,23 @@ -github.com/benbjohnson/clock v1.3.0 h1:ip6w0uFQkncKQ979AypyG0ER7mqUSBdKLOgAle/AT8A= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= -github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8= github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw= -go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/dig v1.17.1 h1:Tga8Lz8PcYNsWsyHMZ1Vm0OQOUaJNDyvPImgbAu9YSc= go.uber.org/dig v1.17.1/go.mod h1:Us0rSJiThwCv2GteUN0Q7OKvU7n5J4dxZ9JKUXozFdE= -go.uber.org/goleak v1.1.11 h1:wy28qYRKZgnJTxGxvye5/wgWr1EKjmUDGYox5mGlRlI= -go.uber.org/multierr v1.6.0 h1:y6IPFStTAIT5Ytl7/XYmHvzXQ7S3g/IeZW9hyZ5thw4= -go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= -go.uber.org/zap v1.23.0 h1:OjGQ5KQDEUawVHxNwQgPpiypGHOxo2mNZsOqTak4fFY= -go.uber.org/zap v1.23.0/go.mod h1:D+nX8jyLsMHMYrln8A0rJjFt/T/9/bGgIhAqxv5URuY= +go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk= +go.uber.org/multierr v1.10.0 h1:S0h4aNzvfcFsC3dRF1jLoaov7oRaKqRGC/pUEJ2yvPQ= +go.uber.org/multierr v1.10.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= +go.uber.org/zap v1.26.0 h1:sI7k6L95XOKS281NhVKOFCUNIvv9e0w4BF8N3u+tCRo= +go.uber.org/zap v1.26.0/go.mod h1:dtElttAiwGvoJ/vj4IwHBS/gXsEu/pZ50mUIRWuG0so= golang.org/x/sys v0.0.0-20220412211240-33da011f77ad h1:ntjMns5wyP/fN65tdBD4g8J5w8n015+iIIs9rtjXkY0= golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= diff --git a/internal/fxclock/clock.go b/internal/fxclock/clock.go index bf1c7058f..6a2bdbcdc 100644 --- a/internal/fxclock/clock.go +++ b/internal/fxclock/clock.go @@ -1,4 +1,4 @@ -// Copyright (c) 2021 Uber Technologies, Inc. +// Copyright (c) 2024 Uber Technologies, Inc. // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal @@ -22,13 +22,13 @@ package fxclock import ( "context" + "sort" + "sync" "time" ) // Clock defines how Fx accesses time. -// The interface is pretty minimal but it matches github.com/benbjohnson/clock. -// We intentionally don't use that interface directly; -// this keeps it a test dependency for us. +// We keep the interface pretty minimal. type Clock interface { Now() time.Time Since(time.Time) time.Duration @@ -56,3 +56,199 @@ func (systemClock) Sleep(d time.Duration) { func (systemClock) WithTimeout(ctx context.Context, d time.Duration) (context.Context, context.CancelFunc) { return context.WithTimeout(ctx, d) } + +// Mock adapted from +// https://github.com/uber-go/zap/blob/7db06bc9b095571d3dc3d4eebdfbe4dd9bd20405/internal/ztest/clock.go. + +// Mock is a fake source of time. +// It implements standard time operations, +// but allows the user to control the passage of time. +// +// Use the [Add] method to progress time. +type Mock struct { + mu sync.RWMutex + now time.Time + + // The MockClock works by maintaining a list of waiters. + // Each waiter knows the time at which it should be resolved. + // When the clock advances, all waiters that are in range are resolved + // in chronological order. + waiters []waiter + waiterAdded *sync.Cond +} + +var _ Clock = (*Mock)(nil) + +// NewMock builds a new mock clock +// using the current actual time as the initial time. +func NewMock() *Mock { + m := &Mock{now: time.Now()} + m.waiterAdded = sync.NewCond(&m.mu) + return m +} + +// Now reports the current time. +func (c *Mock) Now() time.Time { + c.mu.RLock() + defer c.mu.RUnlock() + return c.now +} + +// Since reports the time elapsed since t. +// This is short for Now().Sub(t). +func (c *Mock) Since(t time.Time) time.Duration { + return c.Now().Sub(t) +} + +// Sleep pauses the current goroutine for the given duration. +// +// With the mock clock, this will freeze +// until the clock is advanced with [Add] past the deadline. +func (c *Mock) Sleep(d time.Duration) { + ch := make(chan struct{}) + c.runAt(c.Now().Add(d), func() { close(ch) }) + <-ch +} + +// WithTimeout returns a new context with a deadline of now + d. +// +// When the deadline is passed, the returned context's Done channel is closed +// and the context's Err method returns context.DeadlineExceeded. +// If the cancel function is called before the deadline is passed, +// the context's Err method returns context.Canceled. +func (c *Mock) WithTimeout(ctx context.Context, d time.Duration) (context.Context, context.CancelFunc) { + // Unfortunately, we can't use context.WithCancelCause here. + // Per its documentation (and verified by trying it): + // + // ctx, cancel := context.WithCancelCause(parent) + // cancel(myError) + // ctx.Err() // returns context.Canceled + // context.Cause(ctx) // returns myError + // + // So it won't do for our purposes. + deadline := c.Now().Add(d) + inner, cancelInner := context.WithCancel(ctx) + dctx := &deadlineCtx{ + inner: inner, + cancelInner: cancelInner, + done: make(chan struct{}), + deadline: deadline, + } + ctx = dctx + + c.runAt(deadline, func() { + dctx.cancel(context.DeadlineExceeded) + }) + return ctx, func() { dctx.cancel(context.Canceled) } +} + +type deadlineCtx struct { + inner context.Context + cancelInner func() + + done chan struct{} + deadline time.Time + + mu sync.Mutex // guards err; the rest is immutable + err error +} + +var _ context.Context = (*deadlineCtx)(nil) + +func (c *deadlineCtx) Deadline() (deadline time.Time, ok bool) { return c.deadline, true } +func (c *deadlineCtx) Done() <-chan struct{} { return c.done } +func (c *deadlineCtx) Value(key any) any { return c.inner.Value(key) } + +func (c *deadlineCtx) Err() error { + c.mu.Lock() + err := c.err + c.mu.Unlock() + return err +} + +func (c *deadlineCtx) cancel(err error) { + c.mu.Lock() + if c.err == nil { + c.err = err + close(c.done) + c.cancelInner() + } + c.mu.Unlock() +} + +// runAt schedules the given function to be run at the given time. +// The function runs without a lock held, so it may schedule more work. +func (c *Mock) runAt(t time.Time, fn func()) { + c.mu.Lock() + defer c.mu.Unlock() + + c.waiters = append(c.waiters, waiter{until: t, fn: fn}) + c.waiterAdded.Broadcast() +} + +// AwaitScheduled blocks until there are are at least N +// operations scheduled for the future. +func (c *Mock) AwaitScheduled(n int) { + c.mu.Lock() + defer c.mu.Unlock() + + for len(c.waiters) < n { + c.waiterAdded.Wait() + } +} + +type waiter struct { + until time.Time + fn func() +} + +// Add progresses time by the given duration. +// Other operations waiting for the time to advance +// will be resolved if they are within range. +// +// Side effects of operations waiting for the time to advance +// will take effect on a best-effort basis. +// Avoid racing with operations that have side effects. +// +// Panics if the duration is negative. +func (c *Mock) Add(d time.Duration) { + if d < 0 { + panic("cannot add negative duration") + } + + c.mu.Lock() + defer c.mu.Unlock() + + sort.Slice(c.waiters, func(i, j int) bool { + return c.waiters[i].until.Before(c.waiters[j].until) + }) + + newTime := c.now.Add(d) + // newTime won't be recorded until the end of this method. + // This ensures that any waiters that are resolved + // are resolved at the time they were expecting. + + for len(c.waiters) > 0 { + w := c.waiters[0] + if w.until.After(newTime) { + break + } + c.waiters[0] = waiter{} // avoid memory leak + c.waiters = c.waiters[1:] + + // The waiter is within range. + // Travel to the time of the waiter and resolve it. + c.now = w.until + + // The waiter may schedule more work + // so we must release the lock. + c.mu.Unlock() + w.fn() + // Sleeping here is necessary to let the side effects of waiters + // take effect before we continue. + time.Sleep(1 * time.Millisecond) + c.mu.Lock() + } + + c.now = newTime +} diff --git a/internal/fxclock/clock_test.go b/internal/fxclock/clock_test.go index 560867ecd..677dfde22 100644 --- a/internal/fxclock/clock_test.go +++ b/internal/fxclock/clock_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2021 Uber Technologies, Inc. +// Copyright (c) 2024 Uber Technologies, Inc. // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal @@ -22,44 +22,150 @@ package fxclock import ( "context" + "sync" "testing" "time" - "github.com/benbjohnson/clock" "github.com/stretchr/testify/assert" ) -var _ Clock = clock.Clock(nil) - -// Just a basic sanity check that everything is in order. func TestSystemClock(t *testing.T) { - t.Parallel() - clock := System + testClock(t, System, clock.Sleep) +} + +func TestMockClock(t *testing.T) { + clock := NewMock() + testClock(t, clock, clock.Add) +} - t.Run("Now and Since", func(t *testing.T) { - t.Parallel() +func testClock(t *testing.T, clock Clock, advance func(d time.Duration)) { + now := clock.Now() + assert.False(t, now.IsZero()) - before := clock.Now() - assert.GreaterOrEqual(t, clock.Since(before), time.Duration(0)) + t.Run("Since", func(t *testing.T) { + advance(1 * time.Millisecond) + assert.NotZero(t, clock.Since(now), "time must have advanced") }) t.Run("Sleep", func(t *testing.T) { - t.Parallel() + start := clock.Now() + go advance(1 * time.Millisecond) + clock.Sleep(1 * time.Millisecond) - assert.NotPanics(t, func() { - clock.Sleep(time.Millisecond) - }) + assert.NotZero(t, clock.Since(start), "time must have advanced") }) t.Run("WithTimeout", func(t *testing.T) { - t.Parallel() + ctx, cancel := clock.WithTimeout(context.Background(), 1*time.Millisecond) + defer cancel() + + t.Run("Deadline", func(t *testing.T) { + dl, ok := ctx.Deadline() + assert.True(t, ok, "must have a deadline") + assert.True(t, dl.After(now), "deadline must be in the future") + }) - ctx := context.Background() - ctx, cancel := clock.WithTimeout(ctx, time.Second) + advance(1 * time.Millisecond) + + select { + case <-ctx.Done(): + assert.Error(t, ctx.Err(), "done context must error") + assert.ErrorIs(t, ctx.Err(), context.DeadlineExceeded, + "context must have exceeded its deadline") + + case <-time.After(10 * time.Millisecond): + t.Fatal("expected context to be done") + } + }) + + t.Run("WithTimeout/Value", func(t *testing.T) { + type contextKey string + key := contextKey("foo") + + ctx1 := context.WithValue(context.Background(), key, "bar") + + ctx2, cancel := clock.WithTimeout(ctx1, 1*time.Millisecond) defer cancel() - _, ok := ctx.Deadline() - assert.True(t, ok, "must have deadline") + assert.Equal(t, "bar", ctx2.Value(key), "value must be preserved") }) + + t.Run("WithTimeout/Cancel", func(t *testing.T) { + ctx, cancel := clock.WithTimeout(context.Background(), 1*time.Millisecond) + cancel() + + select { + case <-ctx.Done(): + assert.Error(t, ctx.Err(), "done context must error") + assert.ErrorIs(t, ctx.Err(), context.Canceled, + "context must have been canceled") + + case <-time.After(10 * time.Millisecond): + t.Fatal("expected context to be done") + } + }) +} + +func TestMock_Sleep(t *testing.T) { + clock := NewMock() + + ch := make(chan struct{}) + go func() { + clock.Sleep(2 * time.Millisecond) + close(ch) + }() + + // We cannot advance time until we're certain + // that the Sleep call has started waiting. + // Otherwise, we'll advance that one millisecond, + // and then the Sleep will start waiting for another Advance, + // which will never come. + // + // AwaitScheduled will block until there is at least one + // scheduled event. + clock.AwaitScheduled(1) + + // Adbance only one millisecond, the Sleep should not return. + clock.Add(1 * time.Millisecond) + select { + case <-ch: + t.Fatal("sleep should not have returned") + case <-time.After(1 * time.Millisecond): + // ok + } + + // Avance to the next millisecond, the Sleep should return. + clock.Add(1 * time.Millisecond) + select { + case <-ch: + // ok + case <-time.After(10 * time.Millisecond): + t.Fatal("expected Sleep to return") + } +} + +func TestMock_AddNegative(t *testing.T) { + clock := NewMock() + assert.Panics(t, func() { clock.Add(-1) }) +} + +func TestMock_ManySleepers(t *testing.T) { + const N = 100 + + clock := NewMock() + + var wg sync.WaitGroup + wg.Add(N) + for i := 0; i < N; i++ { + go func() { + defer wg.Done() + + clock.Sleep(1 * time.Millisecond) + }() + } + + clock.AwaitScheduled(N) + clock.Add(1 * time.Millisecond) + wg.Wait() } From d81295331b511de35b89d52ebc7195bf653a9884 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 13 Feb 2024 13:19:13 -0800 Subject: [PATCH 2/7] deadlineCtx.Err: Use defer Co-authored-by: Jacob Oaks --- internal/fxclock/clock.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/fxclock/clock.go b/internal/fxclock/clock.go index 6a2bdbcdc..d15aabddd 100644 --- a/internal/fxclock/clock.go +++ b/internal/fxclock/clock.go @@ -161,9 +161,8 @@ func (c *deadlineCtx) Value(key any) any { return c.inner. func (c *deadlineCtx) Err() error { c.mu.Lock() - err := c.err - c.mu.Unlock() - return err + defer c.mu.Unlock() + return c.err } func (c *deadlineCtx) cancel(err error) { From a874428f19fec9a23db089c4c176535a197fdaed Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 13 Feb 2024 13:22:02 -0800 Subject: [PATCH 3/7] TestMock_ManySleepers: Guard against deadlock --- internal/fxclock/clock_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/internal/fxclock/clock_test.go b/internal/fxclock/clock_test.go index 677dfde22..79a6a8af4 100644 --- a/internal/fxclock/clock_test.go +++ b/internal/fxclock/clock_test.go @@ -167,5 +167,17 @@ func TestMock_ManySleepers(t *testing.T) { clock.AwaitScheduled(N) clock.Add(1 * time.Millisecond) - wg.Wait() + + done := make(chan struct{}) + go func() { + defer close(done) + wg.Wait() + }() + + select { + case <-done: + // ok + case <-time.After(10 * time.Millisecond): + t.Fatal("expected all sleepers to be done") + } } From 5ed3bec0879edd75698c67b2a8306f225c646df0 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 13 Feb 2024 13:22:33 -0800 Subject: [PATCH 4/7] fix typo Co-authored-by: Jacob Oaks --- internal/fxclock/clock_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/fxclock/clock_test.go b/internal/fxclock/clock_test.go index 79a6a8af4..1b22bce13 100644 --- a/internal/fxclock/clock_test.go +++ b/internal/fxclock/clock_test.go @@ -126,7 +126,7 @@ func TestMock_Sleep(t *testing.T) { // scheduled event. clock.AwaitScheduled(1) - // Adbance only one millisecond, the Sleep should not return. + // Advance only one millisecond, the Sleep should not return. clock.Add(1 * time.Millisecond) select { case <-ch: From 400c0256be8a795a120d1fe78847d417bfd3c50f Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 13 Feb 2024 13:27:42 -0800 Subject: [PATCH 5/7] testClock/Sleep: Guard against fast advance The same check we have elsewhere with AwaitScheduled is needed for the end-to-end Clock test. --- internal/fxclock/clock_test.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/internal/fxclock/clock_test.go b/internal/fxclock/clock_test.go index 1b22bce13..22a538a2e 100644 --- a/internal/fxclock/clock_test.go +++ b/internal/fxclock/clock_test.go @@ -50,7 +50,21 @@ func testClock(t *testing.T, clock Clock, advance func(d time.Duration)) { t.Run("Sleep", func(t *testing.T) { start := clock.Now() - go advance(1 * time.Millisecond) + + go func() { + // For the mock clock, there's a chance that advance will be + // too fast and the Sleep will block forever, waiting for + // another advance. The mock clock provides + // AwaitScheduled to help with this. + // + // Since that function is not available on the system clock, + // we'll use upcasting to check for it. + if awaiter, ok := clock.(interface{ AwaitScheduled(int) }); ok { + awaiter.AwaitScheduled(1) + } + + advance(1 * time.Millisecond) + }() clock.Sleep(1 * time.Millisecond) assert.NotZero(t, clock.Since(start), "time must have advanced") From bfc78a90f31f09226bdc723b07209146c3017026 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Tue, 13 Feb 2024 17:54:28 -0800 Subject: [PATCH 6/7] Update internal/fxclock/clock.go --- internal/fxclock/clock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/fxclock/clock.go b/internal/fxclock/clock.go index d15aabddd..c75926bb0 100644 --- a/internal/fxclock/clock.go +++ b/internal/fxclock/clock.go @@ -185,7 +185,7 @@ func (c *Mock) runAt(t time.Time, fn func()) { c.waiterAdded.Broadcast() } -// AwaitScheduled blocks until there are are at least N +// AwaitScheduled blocks until there are at least N // operations scheduled for the future. func (c *Mock) AwaitScheduled(n int) { c.mu.Lock() From 7de231b7b646571f9f9d0fea41084dcc287fb1c4 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 13 Feb 2024 20:10:53 -0800 Subject: [PATCH 7/7] Clarify deadlock avoidance in Mock.AwaitScheduled --- internal/fxclock/clock.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/fxclock/clock.go b/internal/fxclock/clock.go index c75926bb0..8d9e6e1d8 100644 --- a/internal/fxclock/clock.go +++ b/internal/fxclock/clock.go @@ -191,6 +191,13 @@ func (c *Mock) AwaitScheduled(n int) { c.mu.Lock() defer c.mu.Unlock() + // Note: waiterAdded is associated with c.mu, + // the same lock we're holding here. + // + // When we call Wait(), it'll release the lock + // and block until signaled by runAt, + // at which point it'll reacquire the lock + // (waiting until runAt has released it). for len(c.waiters) < n { c.waiterAdded.Wait() }