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

ring: test flake #398

Closed
kevinburkesegment opened this issue Oct 9, 2023 · 3 comments · Fixed by #399
Closed

ring: test flake #398

kevinburkesegment opened this issue Oct 9, 2023 · 3 comments · Fixed by #399

Comments

@kevinburkesegment
Copy link
Contributor

I just had a build fail due to a test failure in the ring package which I believe is unrelated to my changes, as I did not touch the ring package and the ring package doesn't import the packages I changed.

https://drone.grafana.net/grafana/dskit/3189/1/4

--- FAIL: TestSpreadMinimizingTokenGenerator_GetMissingTokens (7.29s)
    spread_minimizing_token_generator_test.go:476: 
        	Error Trace:	/drone/src/ring/spread_minimizing_token_generator_test.go:476
        	Error:      	"[%!s(uint32=2452015680) %!s(uint32=2476020736)]" should have 3 item(s), but has 2
        	Test:       	TestSpreadMinimizingTokenGenerator_GetMissingTokens
FAIL
FAIL	github.com/grafana/dskit/ring	260.251s

In addition it looks like the format string in the error generated by assert.Len is wrong; I raised stretchr/testify#1482 to try to fix that.

@charleskorn
Copy link
Contributor

@duricanikolic knows the most about this test - would you mind taking a look?

@duricanikolic
Copy link
Contributor

I will have a look at this and try to fix it.

@duricanikolic
Copy link
Contributor

I have managed to reproduce the problem: as expected, in an unlucky case,

missingIndex := []int{rand.Intn(tokensPerInstance - 1), rand.Intn(tokensPerInstance - 1), rand.Intn(tokensPerInstance - 1)}

produces array with multiple occurrence of the same number, which caused the problem. I will now fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants