acc: Use time.Duration to configure testserver delay #2531
Conversation
pietern
requested review from
andrewnester,
shreyas-goenka,
denik and
anton-107
as code owners
| @@ -384,10 +384,7 @@ func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsCont | |||
| items := strings.Split(stub.Pattern, " ") | |||
| require.Len(t, items, 2) | |||
| server.Handle(items[0], items[1], func(req testserver.Request) any { | |||
| if stub.DelaySeconds != nil { | |||
| time.Sleep(time.Duration((*stub.DelaySeconds) * float64(time.Second))) | |||
There was a problem hiding this comment.
This type-wrangling triggered me to make the change.
| // Artificial delay to simulate slow responses. | ||
| // Configure as "1ms", "2s", "3m", etc. | ||
| // See [time.ParseDuration] for details. | ||
| Delay time.Duration |
There was a problem hiding this comment.
This should still be a pointer, otherwise you cannot override it from child directory with 0.
There was a problem hiding this comment.
You can put 0 and it won't sleep then
A negative or zero duration causes Sleep to return immediately.
https://pkg.go.dev/time#Sleep
There was a problem hiding this comment.
Yes, I'm aware, I'm talking about merge algorithm. See #2369 for context.
There was a problem hiding this comment.
The ServerStub is a list entry, so there is no way to override it.
Also, note that the pattern and response fields are not pointers.
There was a problem hiding this comment.
Right, I missed the part where it's part of ServerStub, I thought it's a global setting.
| @@ -6,4 +6,4 @@ Response.Body = ''' | |||
| "numProtoSuccess": 2 | |||
| } | |||
| ''' | |||
| DelaySeconds = 5 | |||
| Delay = "5s" | |||
There was a problem hiding this comment.
question - does it need to be changed? Can it still accept plain 5 or 0.01 ?
There was a problem hiding this comment.
No, it only accepts ints, and ints are nanos (the type of time.Duration).
Changes
Use
time.Durationfor the testserver delay. It conveniently zero-initializes, so by default, there is no delay.Why
The TOML library natively supports
time.Duration, so we should use it.