feat(bigtable/cmd/cbt): Add a timeout option #4276
Conversation
jimfulton
requested review from
crwilcox,
kolea2,
tritone and
a team
as code owners
google-cla
bot
added
the
cla: yes
codyoss
changed the title
Note that tableLike is likely to grow as we test more. :)
Except flags, which are untestable. :-[
|
This now has a test! And includes some refactoring that makes some kinds of tests easier. |
| "os" | ||
| ) | ||
|
|
||
| func captureStdout(f func()) string { |
There was a problem hiding this comment.
Document this? Wasn't immediately clear to me what f was for.
There was a problem hiding this comment.
Done.
Note that here, we're mainly preventing spurious print output. In other branches/PRs, we're making assertions about captured stdout.
| @@ -581,7 +607,7 @@ func doCount(ctx context.Context, args ...string) { | |||
| if len(args) != 1 { | |||
| log.Fatal("usage: cbt count <table>") | |||
| } | |||
| tbl := getClient(bigtable.ClientConfig{}).Open(args[0]) | |||
| tbl := getTable(bigtable.ClientConfig{}, args[0]) | |||
There was a problem hiding this comment.
I am having some difficulty fully understanding the need of this refactoring and its relation to the timeout change. Can you describe it a bit?
There was a problem hiding this comment.
I wanted to test that a proper context (with a timeout) was passed to ReadRows.
getTable returns a tableLike object, which simply has ReadRows. (I think I expand it in another PR, IIRC :) )
Normally, getTable calls getClient(clientConf).Open(tableName), but if the global table is set, it returns that.
timeout_test.go sets table to a mock that captures the context passed to ReadRows so we can assert that it was set correctly.
Fixes: #2492