clientv3/integration: add TestBalancerUnderNetworkPartitionWatch #8762
Conversation
we could. |
| defer watchCli.Close() | ||
|
|
||
| // wait for eps[follower] to be pinned | ||
| watchCli.Get(context.Background(), "foo") |
There was a problem hiding this comment.
put a timeout instead of using background.
| t.Fatal("expect open watch channel") | ||
| } | ||
| case <-time.After(3 * time.Second): // enough time to detect leader lose | ||
| t.Fatal("took too long to receive events") |
There was a problem hiding this comment.
took too long to detect leader lost.
| case <-time.After(3 * time.Second): // enough time to detect leader lose | ||
| t.Fatal("took too long to receive events") | ||
| } | ||
| select { |
There was a problem hiding this comment.
we do not need this. this is testing the watch interface behavior.
|
/cc @jpbetz PTAL. this test tests if etcd client is smart enough to when the connected node has no leader (so watch will not pend forever just to receive nothing). |
gyuho
force-pushed
the
partition-test
branch
2 times, most recently
from
3f8aa94
to
4a9bb7d
Compare
Address etcd-io#8762 (comment). Signed-off-by: Gyu-Ho Lee <[email protected]>
|
lgtm. defer to @jpbetz |
gyuho
force-pushed
the
partition-test
branch
2 times, most recently
from
f27844a
to
3319d5c
Compare
Signed-off-by: Gyu-Ho Lee <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #8762 +/- ##
==========================================
- Coverage 76.08% 76.04% -0.05%
==========================================
Files 360 360
Lines 29629 29629
==========================================
- Hits 22543 22530 -13
- Misses 5516 5522 +6
- Partials 1570 1577 +7
Continue to review full report at Codecov.
|
| if err = ev.Err(); err != rpctypes.ErrNoLeader { | ||
| t.Fatalf("expected %v, got %v", rpctypes.ErrNoLeader, err) | ||
| } | ||
| case <-time.After(3 * time.Second): // enough time to detect leader lost |
There was a problem hiding this comment.
Our integration tests are starting to get filled with hard coded wait times that we believe are appropriate for various cases.
To help maintain the tests and keep flakiness to a minimum, should we establish a set of constants that we references from the integration tests instead? E.g instead of 3 here, maybe have a constant like maxLeaderChangeWait. Once we have a reasonable set of constants, then developers don't need to try to figure out what a good number of seconds is each time they write a test, and if we determine we need to increase the number to minimize test flakes based, we can change it in one place...
Just a though, we don't need to do this now. If this seams reasonable, maybe create an issue?
There was a problem hiding this comment.
we should. these random in place non-const timeouts def is not helping anything at all. and we are simply waiting for flakes.
|
Merging. Let's address the random sleeps in the separate issue. Thanks! |
@xiang90 Should we also test when leader becomes isolated as well?
This only tests:
WithRequireLeaderdetects leader lose[EDIT] add test case with leader
#8711