Skip to content

tests: Fix flaky TestSingleClusterPartitionReader_ConsumeAtStartup - #16340

Open
56quarters wants to merge 1 commit into
mainfrom
56quarters/ingest-test
Open

tests: Fix flaky TestSingleClusterPartitionReader_ConsumeAtStartup#16340
56quarters wants to merge 1 commit into
mainfrom
56quarters/ingest-test

Conversation

@56quarters

@56quarters 56quarters commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Fixes a flaky subtest of TestSingleClusterPartitionReader_ConsumeAtStartup that often failed with an unexpected context cancelled error. Avoid this by using the test context to control the producer thread and stopping it when the test ends. This avoids the producer thread trying to write with a cancelled context after the test has ended but before cleanup methods have run.

Which issue(s) this PR fixes or relates to

N/A

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]. If changelog entry is not needed, please add the changelog-not-needed label to the PR.
  • about-versioning.md updated with experimental features.

Fixes a flaky subtest of TestSingleClusterPartitionReader_ConsumeAtStartup
that often failed with an unexpected context cancelled error. Avoid this
by using the test context to control the producer thread and stopping it
when the test ends. This avoids the producer thread trying to write with
a cancelled context after the test has ended but before cleanup methods
have run.

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>
@56quarters 56quarters added the changelog-not-needed PRs that don't need a CHANGELOG.md entry label Aug 10, 2026
@56quarters
56quarters marked this pull request as ready for review August 10, 2026 23:09
@56quarters
56quarters requested a review from a team as a code owner August 10, 2026 23:09

@karlskewes karlskewes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, t.Context() is cancelled just before t.Cleanup(..) runs so the select unblocks.

@pitasi

pitasi commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

please rebase this PR on latest main, otherwise it can't merge — it's blocked on required checks that will never report.

context: #16335 changed the unit test CI matrix and the required checks got renamed (test (N, 10)test (N, 5)). CI runs from before that change only report the old names, so the branch needs to pick up the new workflow from main. sorry for the churn.

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

Labels

changelog-not-needed PRs that don't need a CHANGELOG.md entry flaky-test type/tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants