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

Fix flaky session selection test #2199

Merged
merged 5 commits into from Jan 21, 2022
Merged

Fix flaky session selection test #2199

merged 5 commits into from Jan 21, 2022

Conversation

cyberj0g
Copy link
Contributor

Refactor test to fix test case affecting subsequent test case.

Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

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

Hm its not immediately clear to me why these changes fix the flaky test. I see that we extracted a bunch of logic from the _SelectUnknownSession_RandFreq test into its own function and then call it at the beginning of the test, but it doesn't seem like anything functionally changed in the test. Could you elaborate on how this change fixes the flaky test?

@cyberj0g
Copy link
Contributor Author

cyberj0g commented Jan 21, 2022

@yondonfu I moved test setup, where broadcast sessions are created, to separate function, which is now called twice, for each test case. Before that, if for loop in first test case randomly picked a session with max stake (with about 0.1 chance), next test case, which expects this session to be picked, failed.

Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

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

I moved test setup, where broadcast sessions are created, to separate function, which is now called twice, for each test case. Before that, if for loop in first test case randomly picked a session with max stake (with about 0.1 chance), next test case, which expects this session to be picked, failed.

Ah I missed that second call to the function to create a new selector.

LGTM after squashing the commits!

@cyberj0g cyberj0g merged commit 12bc1f1 into master Jan 21, 2022
@cyberj0g cyberj0g deleted the ip/flaky-tests2 branch January 21, 2022 15:55
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 this pull request may close these issues.

None yet

2 participants