[FTR] Reduce wasted wait in header.waitUntilLoadingHasFinished#271855
Draft
csr wants to merge 3 commits into
Draft
[FTR] Reduce wasted wait in header.waitUntilLoadingHasFinished#271855csr wants to merge 3 commits into
header.waitUntilLoadingHasFinished#271855csr wants to merge 3 commits into
Conversation
The "appear" step in `waitUntilLoadingHasFinished()` polls for the `globalLoadingIndicator` test-subj for 1500ms. Kibana's loading state is itself driven by a 250ms debounce on `http.getLoadingCount$` (`LOADING_DEBOUNCE_TIME` in `core/.../chrome_hooks.ts`), so when a request is in flight the loader becomes visible within ~250ms; when no request is in flight, the indicator never appears and the full 1500ms is wasted. This helper is one of the most-called UI synchronization points in FTR (~3000 call sites across `src/platform/test/`, `x-pack/`, and solutions), and the page is already idle for the majority of those calls (after `navigateToApp`, after `retry.waitFor`, etc.). Reducing the appear-step timeout from 1500ms to 500ms (debounce + 250ms safety buffer) cuts ~1s off every idle invocation while preserving correctness: the `awaitGlobalLoadingIndicatorHidden` step still bounds the overall wait at `defaultFindTimeout * 10` (100s) and is the authoritative "loading finished" signal. Refs elastic#222306 Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Jobs for this PR can be triggered through checkboxes. 🚧
ℹ️ To trigger the CI, please tick the checkbox below 👇
|
header.waitUntilLoadingHasFinished
Contributor
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]
|
Contributor
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#12505[✅] x-pack/platform/test/functional/apps/lens/group1/config.ts: 50/50 tests passed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Tactical reduction of the hard-coded "wait for loading indicator to appear" timeout in
header.waitUntilLoadingHasFinished()from 1500ms to 500ms.This is FTR's most-called UI sync helper (~3000 call sites). On the steady-state path (page already idle), the loader never appears and the full 1500ms is wasted; cutting it to 500ms saves ~1s per idle invocation while preserving correctness.
Refs #222306. Scout has already deprecated the equivalent helper (#246843).
Why this is "stop the bleeding", not the proper fix
The root cause of the wasted time is a misuse pattern:
The proper long-term fix is to migrate callers off
waitUntilLoadingHasFinishedto direct URL navigation + explicitexistOrFail('<unique-element>')checks. That is a much larger, per-suite undertaking — Scout already followed that path in #246843.This PR is a single, narrow change that buys back ~1s per idle call across all existing callers while the proper migration happens in parallel.
Why 500ms is enough
The Kibana global loading indicator is driven by a 250ms debounce on
http.getLoadingCount$:If a request is in flight, the indicator becomes visible within ~250ms. 500ms gives 2x safety margin over the debounce; longer waits are pure overhead when the page is already idle (the common case for this helper, e.g. after
navigateToApp, afterretry.waitFor, etc.).The disappear step (
awaitGlobalLoadingIndicatorHidden, capped atdefaultFindTimeout * 10= 100s) is unchanged and remains the authoritative "loading finished" signal — any rare case where the loader appears later than 500ms is absorbed by that 100s window.Risk assessment
WebDriver is notoriously slow at rendering, so 500ms is not obviously safe without empirical validation. The risk being tested here: whether 500ms is tight enough to occasionally false-negative the appear step on slow CI agents, and whether that cascades into flake (it shouldn't — the disappear step still bounds the overall wait at 100s — but we want to confirm).
To validate, this PR runs the flaky-test-runner across the most timing-sensitive FTR configs that exercise this helper heavily.
Expected savings (if confirmed flake-free)
Test plan
node scripts/eslint --fix— no errors.node scripts/type_check --project src/platform/test/tsconfig.json— passes.node scripts/check_changes.ts— passes.status_pageconfig) — blocked by an environmental Chrome 149 / Kibana CSP incompatibility (inline script violates 'script-src 'report-sample' 'self''in the Kibana page) that also reproduces onmainwithout this change. CI is the authoritative validator.ci:collect-ftr-timingdata confirms per-call savings.Follow-up
The proper fix is migrating callers — direct URL navigation +
existOrFail('<unique-element>'). Happy to file a tracking issue if this PR lands cleanly.