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 TestConcurrentFetchers_fetchSingle #10567

Merged

Conversation

dimitarvdimitrov
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov commented Feb 3, 2025

Why it's falky

The test was failing because the control function was used up by the periodic fetch loop of the started concurrentFetchers. This meant that the test sometimes ran when we don't have a control function registered and didn't inject an error.

At the same time the test cases didn't retain their control functions so that they don't interfere with each other.

What this PR does

This change makes the test cases independent, so we can use KeepControl.

Which issue(s) this PR fixes or relates to

Fixes #9916

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

The test was failing because the control function was used up by the periodic fetch loop of the started `concurrentFetchers`. This meant that the test sometimes ran when we don't have a control function registered and didn't inject an error.

At the same time the test cases didn't retain their control functions so that they don't interfere with each other.

This change makes the test cases independent, so we can use `KeepControl`.

Signed-off-by: Dimitar Dimitrov <[email protected]>
@dimitarvdimitrov dimitarvdimitrov requested a review from a team as a code owner February 3, 2025 11:44
Signed-off-by: Dimitar Dimitrov <[email protected]>
Signed-off-by: Dimitar Dimitrov <[email protected]>
Copy link
Contributor

@narqo narqo left a comment

Choose a reason for hiding this comment

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

🔥

@dimitarvdimitrov dimitarvdimitrov enabled auto-merge (squash) February 6, 2025 09:29
@dimitarvdimitrov dimitarvdimitrov merged commit 9c0e5ba into main Feb 6, 2025
28 checks passed
@dimitarvdimitrov dimitarvdimitrov deleted the dimitar/ingest/fix-flaky-concurrent-fetchers-test branch February 6, 2025 09:36
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.

Flaky TestConcurrentFetchers
2 participants