test: stabilize flaky TestNormalError

[flaky-claw]

What problem does this PR solve?

Issue Number: close #67741

Problem Summary:
Flaky test TestNormalError in br/pkg/streamhelper intermittently fails, so this PR stabilizes that path.

What changed and how does it work?

Root Cause

waitPendingEvents could observe no immediate channel-length progress while listener goroutines still had pending flush events, so Drop() canceled the delivery path and the final merged checkpoint lagged behind cp.

Fix

Waiting for collectCheckpointSpans(t, sub, cp) preserves the original recovery assertion while removing the invalid timing assumption.

Verification

Spec:

• target: br/pkg/streamhelper :: TestNormalError
• strategy: tidb.go_flaky.default
• plan mode: BASELINE_ONLY
• requirements: required case must execute; no skip; repeat count = 1
• execution surface: GO_TEST_WITH_TAGS
• build tags: intest, deadlock
• baseline gates: required_flaky_gate, build_safety_gate, intent_guard_gate
• feedback surface source: baseline_only

Observed result:

• status: failed
• required case executed: yes
• submission decision: ALLOWED
• note: Required flaky case executed during validation.
  Required flaky case was not skipped.
  target_flaky_exact passed.
  timing_about_to_send passed.

Gate checklist:

• target_flaky_exact: PASS
• timing_about_to_send: PASS
• package_failpoint_enabled: SKIPPED
• build: BLOCKED

Commands:

• go test -json -tags=intest,deadlock ./br/pkg/streamhelper -run '^TestNormalError$' -count=1
• GO_FAILPOINTS='github.com/pingcap/tidb/br/pkg/streamhelper/subscription.listenOver.aboutToSend=sleep("5ms")' ./tools/check/failpoint-go-test.sh br/pkg/streamhelper -run '^TestNormalError$' -count=1

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No need to test

  • I checked and no code files have been changed.

Side effects

• Performance regression: Consumes more CPU
• Performance regression: Consumes more Memory
• Breaking backward compatibility

Documentation

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Fixes #67741

Summary by CodeRabbit

• Tests
  • Improved test handling for checkpoint-spanning events.
  • Updated event collection and synchronization before final validation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f9de6712-2025-4fd3-b7c7-86b7a2d15148

📥 Commits

Reviewing files that changed from the base of the PR and between 0b84b15 and 918de91.

📒 Files selected for processing (1)

• br/pkg/streamhelper/subscription_test.go

---

📝 Walkthrough

Walkthrough

TestSubBasic now collects checkpoint-spanning events with collectCheckpointSpans before dropping the subscriber and uses the collected result for its final assertion.

Changes

Subscription test synchronization

Layer / File(s)	Summary
Checkpoint span collection br/pkg/streamhelper/subscription_test.go	TestSubBasic replaces pending-event waiting and manual span merging with collectCheckpointSpans(t, sub, cp) before calling sub.Drop().

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

• pingcap/tidb#67859: Updates the same test’s event collection and subscriber synchronization flow.
• pingcap/tidb#69264: Modifies related subscriber waiting and cleanup logic in the same test file.

Suggested labels: approved, lgtm

Suggested reviewers: ridrisr, yujuncen

Poem

I’m a rabbit with spans in my queue,
Collecting each checkpoint before I’m through.
No pending wait, no manual array,
Then drop the subscriber—clean and fair! 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the main change: stabilizing flaky TestNormalError.
Description check	✅ Passed	The description includes the required issue number, problem summary, change explanation, tests, and release note sections.
Linked Issues check	✅ Passed	The patch addresses #67741 by stabilizing TestNormalError as requested.
Out of Scope Changes check	✅ Passed	The only code change is the test stabilization in br/pkg/streamhelper, with no unrelated additions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.9378%. Comparing base (0b84b15) to head (918de91).

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #70054        +/-   ##
================================================
- Coverage   76.3266%   73.9378%   -2.3889%     
================================================
  Files          2041       2058        +17     
  Lines        559579     578881     +19302     
================================================
+ Hits         427108     428012       +904     
- Misses       131570     150514     +18944     
+ Partials        901        355       -546     

Flag	Coverage Δ
integration	40.9465% <ø> (+1.2412%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	59.8807% <ø> (ø)
parser	∅ <ø> (∅)
br	47.4060% <ø> (-15.3154%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[flaky-claw]

PR: #70054
Confidence: 76
Category: Timing/Eventually consistent (premature span collection before all events delivered)
Diff alignment: Replaces the fragile waitPendingEvents (which returns when events stop arriving, not when checkpoint is reached) + manual sub.Events() merge with collectCheckpointSpans, a proven helper already used by TestSubBasic. The new function drains events non-blockingly via select default and returns only when observed.MinValue() >= checkpoint, directly eliminating the root-cause race.
Evidence: problem-cases confirms TestNormalError as flaky (count=1); index.log locates output at lines 329→368; target-output shows checkpoint mismatch: expected 862, actual 70 — waitPendingEvents returned before all checkpoint-advancing events were delivered, causing early span collection to miss events.
Missing evidence: none
Suggested next steps: The switch to collectCheckpointSpans is a clean improvement over PR #67903's quiet-window approach — it reuses an already-proven helper and directly ties the wait condition to the checkpoint value. Recommend human review; monitor for recurrence in CI.

[ti-chi-bot]

@flaky-claw: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-br-integration-test	918de91	link	true	/test pull-br-integration-test

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RidRisR, YuJuncen

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• br/OWNERS [YuJuncen]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ti-chi-bot]

[LGTM Timeline notifier]

Timeline:

• 2026-07-24 09:51:05.309177351 +0000 UTC m=+1571251.345272407: ☑️ agreed by YuJuncen.
• 2026-07-24 10:00:29.409481033 +0000 UTC m=+1571815.445576089: ☑️ agreed by RidRisR.