Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

snapshot: add "snapshot" restore/save/member-add tests #9198

Merged
merged 2 commits into from
Jan 23, 2018

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Jan 23, 2018

For #9151.

@codecov-io
Copy link

Codecov Report

Merging #9198 into master will increase coverage by 0.12%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9198      +/-   ##
==========================================
+ Coverage   75.75%   75.88%   +0.12%     
==========================================
  Files         363      363              
  Lines       30158    30158              
==========================================
+ Hits        22847    22884      +37     
+ Misses       5704     5672      -32     
+ Partials     1607     1602       -5
Impacted Files Coverage Δ
pkg/transport/timeout_conn.go 80% <0%> (-20%) ⬇️
clientv3/namespace/watch.go 87.87% <0%> (-12.13%) ⬇️
pkg/fileutil/purge.go 73.68% <0%> (-7.9%) ⬇️
clientv3/concurrency/session.go 88.63% <0%> (-4.55%) ⬇️
pkg/testutil/recorder.go 77.77% <0%> (-3.71%) ⬇️
etcdserver/api/v3election/election.go 64.7% <0%> (-2.95%) ⬇️
proxy/httpproxy/director.go 80% <0%> (-2.86%) ⬇️
integration/bridge.go 90.07% <0%> (-2.3%) ⬇️
pkg/schedule/schedule.go 81.69% <0%> (-1.41%) ⬇️
clientv3/op.go 72.56% <0%> (-1.33%) ⬇️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a12eaf...a9e0582. Read the comment docs.

"github.com/coreos/etcd/etcdserver"
)

func TestSnapshotV3RestoreMultiMemberAdd(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably need add a comment on this e2e test. not sure what it tests from the func name.

t.Fatal(err)
}

time.Sleep(3 * time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the sleep here?

t.Fatal(err)
}
defer cli2.Close()
mresp, err := cli2.MemberList(context.Background())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add timeout in context or the test might hang.

}
for i := range kvs {
var gresp *clientv3.GetResponse
gresp, err = cli2.Get(context.Background(), kvs[i].k)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timeout on context


cfg := embed.NewConfig()
cfg.Name = "3"
cfg.InitialClusterToken = "tkn"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tkn probably should be a const, i assume other funcs also use it.

select {
case <-srv.Server.ReadyNotify():
case <-time.After(10 * time.Second):
t.Fatalf("failed to start embed.Etcd")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

failed to start the newly added etcd member

}
defer cli2.Close()

ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably define 3 seconds as request timeout in our pkg/testutil.

k, v string
}

func createSnapshot(t *testing.T, kvs []kv) string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createSnapshotFile

probably also comment that the returned string value is the file name

t.Fatal(err)
}
for i := range kvs {
if _, err = cli.Put(context.Background(), kvs[i].k, kvs[i].v); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

context needs timeout.

@xiang90
Copy link
Contributor

xiang90 commented Jan 23, 2018

LGTM after fixing nits.

Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
@gyuho gyuho merged commit 131da29 into etcd-io:master Jan 23, 2018
@gyuho gyuho deleted the snapshot-test branch January 23, 2018 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants