Skip to content

[Custom threshold] Freeze timers so error-callout test ignores a11y announcer duplicate - #290453

Open
kibanamachine wants to merge 1 commit into
mainfrom
fix/flaky-253621-custom-threshold-error-announcer-5b51f62e7578babe
Open

[Custom threshold] Freeze timers so error-callout test ignores a11y announcer duplicate#290453
kibanamachine wants to merge 1 commit into
mainfrom
fix/flaky-253621-custom-threshold-error-announcer-5b51f62e7578babe

Conversation

@kibanamachine

Copy link
Copy Markdown
Contributor

Fixes #253621

Summary

  • The custom_threshold "should show an error message when searchSource throws an error" test read the whole error-callout subtree with .text() and asserted it equals the title + text + button exactly.
  • The callout (KbnDangerCalloutEuiCallOut) is rendered with announceOnMount, so EuiCallOut also mounts an EuiLiveAnnouncer screen-reader live region that populates a duplicate of the callout's title/text ~50ms after mount, via a real setTimeout.
  • Under CI parallel load, >50ms elapses between mount and the assertion, so the live region has populated and the .text() read scoops up its duplicate — doubling the string and failing the exact-match assertion.
  • This patch freezes timers for that one test so the live region never populates during the assertion, while keeping nextTick real so the async setup() update still flushes. The assertion (and its coverage) is unchanged.

Context

  • No failed-test investigator root-cause comment exists on the issue — only automated kibanamachine "New failure" notifications — so this was investigated from scratch against current main.
  • Confirmed the mechanism on current main: forcing a 100ms wait after mount makes the callout's .text() include the live region's role="status" aria-live="polite" node (Error fetching search source, Could not locate that data view (id: )), doubling the asserted string — the same failure family reported on the issue.
  • The live region is intentional a11y behavior (a delayed screen-reader announcement), not part of the synchronously rendered visible error the test means to verify, so removing its nondeterministic injection is a correctness fix, not a weakened assertion.
  • Failures recur across kibana-on-merge on main, 9.3, 9.4, and 9.5 (and one kibana-fips run), from 2026-02 through 2026-09 — most recently main (2026-09-03) and 9.5 (2026-09-02).
Verification

Verified locally

  • ✅ Passed: node scripts/eslint x-pack/solutions/observability/plugins/observability/public/components/custom_threshold/custom_threshold_rule_expression.test.tsx
  • ✅ Passed: node scripts/jest .../custom_threshold_rule_expression.test.tsx — 25/25 runs passed with the fix.
  • ✅ Mechanism reproduced: on the unpatched test, forcing a 100ms real wait after mount deterministically produces the doubled string (the .text() read captures the EuiLiveAnnouncer role="status" duplicate).

Not verified locally

  • The natural race requires >50ms of wall-clock to elapse between mount and the assertion, which only happens reliably under CI parallel load; it does not reproduce in a local loop (an unpatched local loop passes 25/25 and would prove nothing), so the doubling was demonstrated by forcing the timer instead.
  • Behavior under real CI parallel load was not exercised locally.

Note

Requested by @csr. Share feedback in #kibana-qa. Mention @copilot to make quick changes.

Generated by Flaky Test Fixer for #253621 · claude · opus · 617.8 AIC · ⌖ 60.2 AIC · ⊞ 14.5K ·

Fixes #253621

The "should show an error message when searchSource throws an error" test
reads the whole error-callout subtree via `.text()`. Because the callout uses
`announceOnMount`, EuiCallOut renders an EuiLiveAnnouncer screen-reader live
region that populates a title/text duplicate ~50ms after mount (a real
setTimeout). Under CI parallel load the timer fires before the assertion, so
the read captures the duplicate and the string doubles.

Freezing timers for this test keeps that live region empty while leaving
`nextTick` real so the async setup update still flushes.

Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
@infra-vault-gh-plugin-prod

Copy link
Copy Markdown
🤖 Jobs for this PR can be triggered through checkboxes. 🚧

ℹ️ To trigger the CI, please tick the checkbox below 👇

  • Click to trigger kibana-pull-request for this PR!
  • Click to trigger kibana-deploy-project-from-pr for this PR!
  • Click to trigger kibana-deploy-cloud-from-pr for this PR!
  • Click to trigger kibana-entity-store-performance-from-pr for this PR!
  • Click to trigger kibana-storybooks-from-pr for this PR!

@github-actions github-actions Bot added backport:all-open Backport to all branches that could still receive a release flaky-fix-check:skipped Flaky fix verifier: runner can't verify this fix (e.g. no Jest support) release_note:skip Skip the PR/issue when compiling release notes labels Sep 11, 2026
@kibanamachine

Copy link
Copy Markdown
Contributor Author

⏭️ Flaky-fix verification skipped

This is a Jest-only change, so the /flaky runner can't add signal — required CI is the whole verdict. Applied release_note:skip and backport:all-open.

Why the flaky test runner wasn't used

The /flaky runner accepts only FTR and Scout configs. This PR only touches the Jest test custom_threshold_rule_expression.test.tsx, which the fixer already validated by local repetition (25/25). Freezing timers for the one test removes the delayed EuiLiveAnnouncer duplicate the .text() read was scooping up — a deterministic root-cause fix, so a single required-CI pass is a reliable verdict.

How release-note and backport labels were chosen

Applied release_note:skip because the change only edits a test file — no user-facing behavior changes.

  • v9.5.4 → included; the targeted test and the a11y announcer mechanism exist identically and the patch applies unchanged.
  • v9.4.7 → included; same file and anchors, patch applies unchanged.
  • v8.19.22 → included; same targeted test and beforeEach/test anchors present, patch applies unchanged.

Generated by Flaky Fix Verifier for #290453 · claude · opus · 174.4 AIC · ⌖ 60.3 AIC · ⊞ 15.8K ·

@kibanamachine
kibanamachine marked this pull request as ready for review September 11, 2026 08:09
@kibanamachine
kibanamachine requested a review from a team as a code owner September 11, 2026 08:09
@kibanamachine
kibanamachine enabled auto-merge (squash) September 11, 2026 08:09
@kibanamachine

Copy link
Copy Markdown
Contributor Author

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

✅ unchanged

Test Failures

  • [job] [logs] Jest Integration Tests #2 / Inbound events HTTP API creates an inbound webhook, accepts a hub POST, and rejects the old token after rotate
  • [job] [logs] Scout Lane #8 - stateful-classic / default / local-stateful-classic - pingList query - returns a list of pings for the date range and given size
  • [job] [logs] FTR Configs #117 / X-Pack Accessibility Tests - Group 2 ml Accessibility for user with full ML access with data loaded index data visualizer select index pattern page

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

Labels

backport:all-open Backport to all branches that could still receive a release flaky-fix-check:skipped Flaky fix verifier: runner can't verify this fix (e.g. no Jest support) flaky-test-fixer Automated PR created by the flaky test fixer workflow release_note:skip Skip the PR/issue when compiling release notes

Projects

None yet

1 participant