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

integration: watch cancel test #4223

Merged
merged 1 commit into from
Jan 16, 2016
Merged

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Jan 15, 2016

Related #4216.

@gyuho gyuho mentioned this pull request Jan 15, 2016
10 tasks
}{
// one key, one put, one watch, one cancellation
{
[]*pb.PutRequest{
Copy link
Contributor

Choose a reason for hiding this comment

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

let's make these requests one line when possible to improve readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

@gyuho gyuho force-pushed the watch_cancel_test branch from db95995 to f37bc72 Compare January 16, 2016 00:04

wcreatedN := 0
for _, wreq := range tt.wreqs {
if wreq == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

well... why do we want to put a nil into wreqs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I set up the tests, with each put, watch request, watch response having exact the same number of elements
so that I can run each action by index and match event by index. Do you have any suggestion? I need nil to fill
those slice. And if wreq is nil it doesn't send watch request.

@gyuho gyuho force-pushed the watch_cancel_test branch 2 times, most recently from fc9ca17 to 6982a0c Compare January 16, 2016 00:38
@gyuho
Copy link
Contributor Author

gyuho commented Jan 16, 2016

@xiang90 PTAL. Just simplified. Thanks,

t.Fatalf("wAPI.Watch error: %v", err)
}

// send create WatchRequest
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this comment.

@gyuho gyuho force-pushed the watch_cancel_test branch from 6982a0c to 64ffe60 Compare January 16, 2016 01:01
@gyuho
Copy link
Contributor Author

gyuho commented Jan 16, 2016

@xiang90 Just fixed comments. Thanks,

@xiang90
Copy link
Contributor

xiang90 commented Jan 16, 2016

LGTM

@gyuho
Copy link
Contributor Author

gyuho commented Jan 16, 2016

Test all passed. I will quickly fix the govet shadow issue in the same commit and merge after second CI passes.

@gyuho gyuho force-pushed the watch_cancel_test branch from 64ffe60 to 2535509 Compare January 16, 2016 01:16
gyuho added a commit that referenced this pull request Jan 16, 2016
@gyuho gyuho merged commit 22dd738 into etcd-io:master Jan 16, 2016
@gyuho gyuho deleted the watch_cancel_test branch January 31, 2016 00:44
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.

2 participants