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

Fix controller unittests #7867

Merged
merged 1 commit into from May 7, 2015

Conversation

Projects
None yet
3 participants
@bprashanth
Member

bprashanth commented May 6, 2015

Make TestRCExpectations (#7860) use LoadInt. I think the watch failure is just a victim here because it needs to start multiple goroutines and their timings didn't quite line up within 100ms. It already uses a fake listwatcher, so suggestions other than just increasing its timeout are welcome.

@@ -124,9 +124,6 @@ func (r *RCExpectations) ExpectDeletions(rc *api.ReplicationController, dels int
// Decrements the expectation counts of the given rc.
func (r *RCExpectations) lowerExpectations(rc *api.ReplicationController, add, del int) {
if podExp, exists, err := r.GetExpectations(rc); err == nil && exists {
if podExp.add > 0 && podExp.del > 0 {

This comment has been minimized.

@bprashanth

bprashanth May 6, 2015

Member

this isn't worth the loadint.
what can go wrong here, is go just being panicky? im ok with these being inaccurate as i'm just using them for logging. The memory isn't goint to get deallocated because these are members in a struct I have a ref to right?

@bprashanth

bprashanth May 6, 2015

Member

this isn't worth the loadint.
what can go wrong here, is go just being panicky? im ok with these being inaccurate as i'm just using them for logging. The memory isn't goint to get deallocated because these are members in a struct I have a ref to right?

This comment has been minimized.

@lavalamp

lavalamp May 6, 2015

Member

Yeah the memory won't get deallocated. It's complaining because it's technically a race (imagine a system where 64 bit ints took two words).

@lavalamp

lavalamp May 6, 2015

Member

Yeah the memory won't get deallocated. It's complaining because it's technically a race (imagine a system where 64 bit ints took two words).

@lavalamp

This comment has been minimized.

Show comment
Hide comment
@lavalamp

lavalamp May 6, 2015

Member

Fixes #7860

LGTM

Member

lavalamp commented May 6, 2015

Fixes #7860

LGTM

@lavalamp lavalamp added the lgtm label May 6, 2015

lavalamp added a commit that referenced this pull request May 7, 2015

@lavalamp lavalamp merged commit 1177adf into kubernetes:master May 7, 2015

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Shippable Shippable builds completed
Details
cla/google All necessary CLAs are signed

@bprashanth bprashanth deleted the bprashanth:testWatchControllers branch Oct 26, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment