test: write integration test for SessionPool #1442
Conversation
Make sure the low-level functions to make async calls actually work, the value of unit tests for these is limited (we would be re-testing the retry loop, but we have tests for that). An integration test verifies we are filling the protos with the right data. Note that some of these functions are not exercised in the other integration tests because they run too infrequently.
googlebot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #1442 +/- ##
==========================================
+ Coverage 93.55% 95.58% +2.03%
==========================================
Files 193 192 -1
Lines 15852 15793 -59
==========================================
+ Hits 14830 15096 +266
+ Misses 1022 697 -325
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This change LGTM, but I was hoping to let @mr-salty approve too since he's the session-pool-person.
Reviewable status: 0 of 4 files reviewed, all discussions resolved (waiting on @devbww, @devjgm, @mr-salty, and @scotthart)
There was a problem hiding this comment.
Reviewed 2 of 4 files at r1.
Reviewable status: 2 of 4 files reviewed, 1 unresolved discussion (waiting on @coryan, @devbww, @mr-salty, and @scotthart)
google/cloud/spanner/internal/session_pool.h, line 100 at r1 (raw file):
std::shared_ptr<SpannerStub> GetStub(Session const& session); // Asynchronous calls used to maintain the pool.
These are not part of the SessionPool API; I'd prefer to keep them private and use friendship.
Alternatively, they could be factored out to another class - looks like the only member variables they access are the policies which could be passed in.
There was a problem hiding this comment.
PTAL
Reviewable status: 1 of 4 files reviewed, 1 unresolved discussion (waiting on @devbww, @mr-salty, and @scotthart)
google/cloud/spanner/internal/session_pool.h, line 100 at r1 (raw file):
Previously, mr-salty (Todd Derr) wrote…
These are not part of the
SessionPoolAPI; I'd prefer to keep them private and use friendship.Alternatively, they could be factored out to another class - looks like the only member variables they access are the policies which could be passed in.
I kept then private and introduced a friend class.
There was a problem hiding this comment.
Reviewed 1 of 4 files at r1, 2 of 2 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @devbww and @scotthart)
…-cpp-spanner#1442) Make sure the low-level functions to make async calls actually work, the value of unit tests for these is limited (we would be re-testing the retry loop, but we have tests for that). An integration test verifies we are filling the protos with the right data. Note that some of these functions are not exercised in the other integration tests because they run too infrequently.
Make sure the low-level functions to make async calls actually work,
the value of unit tests for these is limited (we would be re-testing
the retry loop, but we have tests for that). An integration test
verifies we are filling the protos with the right data. Note that
some of these functions are not exercised in the other integration
tests because they run too infrequently.
This change is