Skip to content

[Trusted Apps] Fix flaky invalid-hash form test by removing async input - #290719

Open
kibanamachine wants to merge 1 commit into
mainfrom
fix/flaky-286496-trusted-apps-hash-sync-input-cf20b8f5c047e232
Open

[Trusted Apps] Fix flaky invalid-hash form test by removing async input#290719
kibanamachine wants to merge 1 commit into
mainfrom
fix/flaky-286496-trusted-apps-hash-sync-input-cf20b8f5c047e232

Conversation

@kibanamachine

Copy link
Copy Markdown
Contributor

Fixes #286496

Summary

  • should validate invalid Hash value timed out at 5000 ms (and later failed to find [1] Invalid hash value) because it drove the value through slow char-by-char userEvent.type inside an async act, unlike its sibling tests which set values synchronously.
  • Replaces the async input with a synchronous setTextFieldValue edit (which marks the field visited via blur) plus an explicit controlled-item update, then asserts — so there is no async step to exceed the budget.
Runtime vs. 5s budget Passed Avg Max
Before fix 25/25 0.6s 0.7s
After fix 25/25 0.4s 0.5s

Context

  • The investigator's analysis correctly identified the async userEvent.type as the cost, but its proposed drop-in replacement (setTextFieldValue(...); rerenderWithLatestProps();) fails 0/25 locally. rerenderWithLatestProps() reads latestUpdatedItem, and the form's useEffect(() => processChanged(), [processChanged]) re-fires the moment hasFormChanged flips falsetrue on the first edit — calling the test's onChange with the still-empty item prop and clobbering latestUpdatedItem back to empty. userEvent.type masked this only because its 2nd–8th keystrokes fire after hasFormChanged is already true; the sibling should validate multiple errors in form avoids it because its AND-click primes hasFormChanged first. This fix therefore departs from the proposed patch: it supplies the controlled item explicitly (as a real parent would) instead of relying on the clobbered round-trip value.
  • Product validation isn't broken: validateValues still runs on the invalid-hash item via that same useEffect and produces the error; the identical assertion runs synchronously and reliably in the sibling at the bottom of the same describe.
  • Failures were on kibana-on-merge - main (first failure build 107204; reopened on build 109576) — a Jest timeout under CI parallel load.
Verification

Verified locally

  • ✅ Passed: node scripts/eslint <file>
  • ✅ Passed: node scripts/jest <file> -t "should validate invalid Hash value": 25/25 passed before the fix (avg 631ms, max 690ms) and 25/25 after (avg 438ms, max 480ms); the ~30% lower average confirms the char-by-char typing cost was removed rather than the wait merely re-tuned.

Not verified locally

  • The reported flake is a timeout under CI parallel load, which does not reproduce on this runner (the unpatched test passed 25/25 locally), so the loops measure the runtime reduction rather than reproducing the failure.

Note

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

Generated by Flaky Test Fixer for #286496 · claude · opus · 801.7 AIC · ⌖ 60.4 AIC · ⊞ 14.5K ·

Replace the char-by-char userEvent.type in an async act with a synchronous
field edit plus an explicit controlled-item update, so there is no async step
to exceed the 5s Jest budget under CI parallel load.

Fixes #286496

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:version Backport to applied version labels 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 v9.4.7 v9.5.4 labels Sep 12, 2026
@kibanamachine

Copy link
Copy Markdown
Contributor Author

⏭️ Flaky-fix verification skipped

The /flaky runner only accepts FTR and Scout configs, and this PR only touches a Jest/RTL test (form.test.tsx), so required CI is the whole verdict — the fixer already verified the fix locally (25/25). Applied release_note:skip and backport:version (v9.5.4, v9.4.7).

Why the flaky test runner wasn't used

The /flaky runner accepts only FTR and Scout configs. This PR only changes the Jest test x-pack/solutions/security/plugins/security_solution/public/management/pages/trusted_apps/view/components/form.test.tsx, replacing an async userEvent.type-inside-act step with a synchronous edit plus an explicit controlled-item update. That removes the async cost deterministically, so there is no timing element left for repeated runs to validate — required CI is sufficient signal.

How release-note and backport labels were chosen

Applied release_note:skip because this is a test-only change with no user-facing effect.

  • v9.5.4 → included; has the identical unpatched flaky async test and every helper the patch uses, so the patch applies without adaptation.
  • v9.4.7 → included; identical to 9.5, patch applies without adaptation.
  • v8.19.22 → excluded; the test is already synchronous there and not flaky, so the async code this patch fixes does not exist on that branch.

Generated by Flaky Fix Verifier for #290719 · claude · opus · 205.3 AIC · ⌖ 61.5 AIC · ⊞ 15.8K ·

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

Copy link
Copy Markdown
Contributor Author

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

✅ unchanged

Test Failures

  • [job] [logs] FTR Configs #71 / lens app - group 1 lens chart style settings Area/Line pointVisibility "before all" hook in "Area/Line pointVisibility"
  • [job] [logs] FTR Configs #52 / lens serverless - group 1 - subgroup 1 lens smokescreen tests should transition from a multi-layer stacked bar to treemap chart using suggestions

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

Labels

backport:version Backport to applied version labels 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 v9.4.7 v9.5.4

Projects

None yet

1 participant