ge/cmd test #28
ge/cmd test #28
Conversation
gesterhuizen force-pushed the ge/cmd-test branch 2 times, most recently from 1d31885 to d8eb330 Compare client.go Outdated
| return DefaultClient.Query(ctx, cmd, args...) | ||
| } | ||
| | ||
| type MultiError []error |
There was a problem hiding this comment.
❤️ this!
However, should this type be unexported?
There was a problem hiding this comment.
Thanks :)
I exported it because it's declared in package redis but used in package main.
Is there a better way to do this ?
There was a problem hiding this comment.
The type implement the errror interface, you can change the signature of your functions to return error instead of MultiError and is should just work I think.
There was a problem hiding this comment.
It should work, but then we lose the additional errors. However, given that we don't use it anywhere (except to stringify the error), I think that's a fair compromise. We can always expose it later. Will change it.
There was a problem hiding this comment.
👍
Errors are rarely reported as concrete types in Go, instead the error interface is used. If we wanted to give access to individual errors we could expose it as an interface that the error may implement, for example something like this:
func (err multiError) Causes() []error { return ([]error)(err) }if causes, ok := err.(interface{ Causes() []error }); ok { for _, err := range causes { // ... } }That way we have a lose coupling between the error producer and consumer and we keep the public API short.
There was a problem hiding this comment.
That's a nice elegant solution. I fixed this to return error now.
client.go Outdated
| } | ||
| | ||
| // ReadTestPattern reads a test pattern (previously writen using WriteTestPattern) from a Redis client and returns hit statistics. | ||
| func ReadTestPattern(client *Client, n int, keyTempl string, timeout time.Duration) (numHits int, numMisses int, numErrors int, err MultiError) { |
There was a problem hiding this comment.
Do these function belong to the redistest subpackage?
There was a problem hiding this comment.
They're used from main as the test subcommand uses them.
So, basically, the redis library exposes functionality anyone (including us when we write red) can use to write these tests.
Happy for suggestions to make it better.
There was a problem hiding this comment.
If you put it in the redistest subpackage it's available from everywhere else, you'd call redistest.ReadTestPattern instead of redis.ReadTestPattern ;)
cmd/red/test.go Outdated
| | ||
| for i := 0; config.Runs == 0 || i < config.Runs; i++ { | ||
| logger.Printf("Starting run #%d.", i) | ||
| transport := &redis.Transport{} |
There was a problem hiding this comment.
You can decorate the transport with the redisstats.NewTransport(redis.RoundTripper) function, so your client will magically start producing more metrics ;)
Call it like this:
transport := redisstats.NewTransport(&redis.Transport{})`There was a problem hiding this comment.
Fancy :) Will do that.
gesterhuizen force-pushed the ge/cmd-test branch 3 times, most recently from 15c9f42 to 41ee2e4 Compare 
…and in test mode.
gesterhuizen force-pushed the ge/cmd-test branch from 41ee2e4 to a57ad38 Compare 
gesterhuizen force-pushed the ge/cmd-test branch from a57ad38 to f8ef293 Compare 
| Merging. |
3 participants
Add a
testsub-command to theredcommand which runs a test and emits test metrics. Did some ad-hoc tests on k8s and it seems happy.