Changes to support E2E tests with confirmations, where requests might be slow #960
Conversation
… be longer Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
peterbroadhurst
requested review from
nguyer,
awrichar and
shorsher
as code owners
| github.com/hyperledger/firefly-common v0.1.19-0.20220811200324-b6c4c44d2d0d h1:9s1MI4RIfKgftrGQ6sI2gvQBEQElq2CQOs3uaiB44s8= | ||
| github.com/hyperledger/firefly-common v0.1.19-0.20220811200324-b6c4c44d2d0d/go.mod h1:MNbaI2spBsdZYOub6Duj9xueE7Qyu9itOmJ4vE8tjYw= | ||
| github.com/hyperledger/firefly-common v0.1.20 h1:0dShkjlIShyBxkXRmu3vLmpEK6xrqmfc8GhF6k0Vgbg= | ||
| github.com/hyperledger/firefly-common v0.1.20/go.mod h1:gMlv4Iy5JjnzXmSEdb+tWVDIc/2GhL9MRcgNX+VmI4M= |
There was a problem hiding this comment.
I don't understand go.sum but why are there so many firefly-common entries??
There was a problem hiding this comment.
I was testing along the way - it seems to just add in each one you use, and never remove it
There was a problem hiding this comment.
e.g. I was doing go get github.com/hyperledger/firefly-common@b6c4c44d2d0dc4b1c511aab629facf67113e3708 from branches in common, to test my changes
There was a problem hiding this comment.
Decision: Will do this in next PR, to keep the pipeline moving as this is way into the E2Es
| if _, isReader := req.Body.(io.Reader); !isReader { | ||
| b, _ = json.Marshal(req.Body) | ||
| } | ||
| l.Logf("%s: ==> %s %s %s: %s", fftypes.Now(), req.Method, req.URL, req.QueryParam, string(b)) |
There was a problem hiding this comment.
Feels a little odd that we add timestamps inside these logs (shouldn't that be a higher-level job delegated to the logging framework for all logs?)
There was a problem hiding this comment.
Yes, but I couldn't find an option in the Go test framework to make t.Logf add timestamps... so this is a workaround that didn't mean changing every log line
| @@ -223,7 +223,7 @@ func checkObject(t *testing.T, expected interface{}, actual interface{}) bool { | |||
| } | |||
|
|
|||
| func VerifyAllOperationsSucceeded(t *testing.T, clients []*client.FireFlyClient, startTime time.Time) { | |||
| tries := 3 | |||
| tries := 30 | |||
| delay := 2 * time.Second | |||
There was a problem hiding this comment.
60 seconds seems quite long
There was a problem hiding this comment.
I'm happy to reduce - I think I need 30seconds with 2 confirmations, obviously more with more confirmations
There was a problem hiding this comment.
OK, if we're sure 30 seconds is "happy path" in this world, 60 seems reasonable. It does make the suite take quite a long time then, right?
There was a problem hiding this comment.
Decision: Leave at 60 for now
Biggest change is to the HTTP Server to allow requests longer than 15s to complete without the client getting an EOF.
See hyperledger/firefly-common#31