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

Testing Refactor #34

Merged
merged 8 commits into from
Aug 15, 2014
Merged

Testing Refactor #34

merged 8 commits into from
Aug 15, 2014

Conversation

yichengq
Copy link

No description provided.

lead, _ := cl.Leader()
cl.Node(int(lead)).Stop()
// wait for leader election timeout
time.Sleep(cl.Node(0).e.tickDuration * defaultElection * 2)

Choose a reason for hiding this comment

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

Why are you sleeping? Please find a way to not sleep.

@bmizerany
Copy link

Let's find a way to remove all of the sleeps soon. We can do that in another PR though. Is this ready to merge?

@xiang90
Copy link

xiang90 commented Aug 14, 2014

@bmizerany Almost ready for merge. We can get rid of the timing. But need some extra work.
This pr is for cleaning up the tests:

  1. Remove unnecessary tests
  2. Remove unnecessary initialization functions for creating testing cluster
  3. Prepare for random generated node Id (perviously we had a bad assumption that the NodeId is from 0 -> k; we have fixed that in all raft tests.)

@yichengq
Copy link
Author

@xiangli-cmu refreshed.

@xiang90
Copy link

xiang90 commented Aug 15, 2014

@unihorn i think we have achieved the 3 goals mentioned in this pr. lgtm.

@xiang90
Copy link

xiang90 commented Aug 15, 2014

@unihorn This needs to be rebased.

@xiang90 xiang90 added the hot label Aug 15, 2014
@bmizerany
Copy link

Okay. I haven't looked over 100% of it, so I'm defering to you two on this. LGTM.

yichengq added a commit that referenced this pull request Aug 15, 2014
@yichengq yichengq merged commit 7076ffa into raft Aug 15, 2014
@bmizerany bmizerany deleted the testing2 branch August 15, 2014 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants