القائمة
Review Scout UI/API tests (including Scout test migrations) for best practices, reuse, parity, and server config hygiene.
يُحدَّد Prompt الذي يراجع المصدر أولًا بشكل افتراضي. يمكنك التبديل إلى أمر مباشر أو تنزيل نسخة محلية.
اقرأ SKILL.md وأي ملفات مرافقة يعرضها SkillsMP قبل أن تقرر التثبيت.
التثبيت باستخدام Codex أو Claude انسخ هذا Prompt والصقه في Codex أو Claude أو مساعد آخر ليراجع صفحة Skill ويثبّتها لك.
يتجاوز الأمر المباشر Prompt المخصّص للمراجعة. افحص المصدر قبل تشغيله.
npx skills add https://github.com/elastic/kibana --skill scout-best-practices-reviewerيبقى الأمر في سطر واحد. مرّر أفقيًا لمراجعته كاملًا قبل النسخ.
تفضّل نسخة محلية؟ نزّل الملفات المتاحة حاليًا لدى SkillsMP.
استنادا إلى تصنيف SOC المهني
عرض SKILL.md
| name | scout-best-practices-reviewer |
| description | Review Scout UI/API tests (including Scout test migrations) for best practices, reuse, parity, and server config hygiene. |
Perform a static PR review of Scout UI and API test files (*.spec.ts) against Scout best practices and existing Scout abstractions (fixtures, page objects, API helpers). Produce actionable, PR-review-ready feedback that pushes for reuse over one-off implementations.
Solution-specific skills may extend this skill with additional review criteria. Check if one exists for your solution (e.g., Security Solution has one at <plugin>/.agents/skills/scout-best-practices-reviewer/). Run the general review first, then apply solution-specific checks.
Important: Do not post GitHub comments unless explicitly stated.
Changed *.spec.ts files (and imported helpers/fixtures).
test / spaceTest (usually in **/test/scout/ui/**).apiTest (usually in **/test/scout/api/**).Neighboring Scout code in the same plugin/solution (existing specs + test/scout/**/fixtures/**) to spot reuse opportunities and avoid duplicating helpers.
Removed/previous tests (if this is a migration) to verify behavior parity.
Scout docs (open only what you need — best practices are split by test type so you can skip the irrelevant half):
docs/extend/testing/scout-best-practices.mddocs/extend/testing/ui-best-practices.mddocs/extend/testing/api-best-practices.mddocs/extend/testing/scout.md, docs/extend/testing/fixtures.mddocs/extend/testing/page-objects.md, docs/extend/testing/api-services.mddocs/extend/testing/write-ui-tests.md, docs/extend/testing/write-api-tests.mddocs/extend/testing/api-auth.md, docs/extend/testing/browser-auth.md, docs/extend/testing/parallelism.md, docs/extend/testing/deployment-tags.md, docs/extend/testing/a11y-checks.md, docs/extend/testing/debugging.md, docs/extend/testing/run-scout-tests.mdRule of thumb: always read the general best practices, then open only the UI-specific file for UI reviews or the API-specific file for API reviews. If a PR mixes UI and API specs, open both.
Work through these numbered checks in order, one at a time, before the general checklist — don't batch or skip. They're the highest-priority findings: a genuine hit almost always means the PR must change before merge. Each row links to the canonical public guidance — read the linked section before flagging, and cite it in the comment.
| # | Critical check | Fires when the PR… | Flag / ask → see |
|---|---|---|---|
| 1 | Custom server config earns its keep | adds or updates a config set (files under .../kbn-scout/.../config_sets/<name>/**, a new test/scout_<name>/ dir, or new --serverConfigSet) | Setting is runtime-toggleable → move it to apiServices.core.settings(...). Duplicates an existing set's purpose/args → reuse that set, or ask its owners to extend it. → docs/extend/testing/feature-flags.md, docs/extend/testing/scout-best-practices.md#prefer-runtime-feature-flags |
| 2 | Spec lives where selective testing runs it | adds or moves a spec that drives another plugin's app/UI (page.gotoApp('<other>'), another solution's data-test-subj, tags, or nav) | Host package doesn't own or kbn_references-depend on the code under test → relocate the spec to the owning plugin so PR CI actually runs it. → docs/extend/testing/scout-best-practices.md#keep-tests-close-to-source-code |
| 3 | Right test type, 100% justified | adds or migrates a UI or API spec, or an FTR/Cypress→Scout migration | UI asserts data/logic only, or drives a pure local UI toggle already covered by a Jest unit/RTL test → move down the pyramid or remove. "It works / it's easier / that's how the FTR did it" isn't justification. → docs/extend/testing/scout-best-practices.md#pick-the-right-test-type, docs/extend/testing/migrate-tests.md#dont-migrate-blindly |
| 4 | Migration parity — no major coverage loss | removes or replaces FTR/Cypress tests alongside new or changed Scout specs | Flag only major changes or drops: removed scenarios, dropped roles/error paths or classic/serverless coverage, weakened assertions or lost side-effect checks. Skip benign deltas — don't post an FYI parity map. Depth in Migration parity analysis below. → docs/extend/testing/migrate-tests.md#dont-migrate-blindly |
docs/extend/testing/fixtures.md + local test/scout/**/fixtures)@kbn/scout, solution Scout packages, and plugin-local test/scout/**) before suggesting brand-new helpersChecklist items are tagged with the document they're detailed in:
docs/extend/testing/scout-best-practices.md (applies to both UI and API tests)docs/extend/testing/ui-best-practices.mddocs/extend/testing/api-best-practices.mdOpen only the docs relevant to the test type(s) under review.
pageObjects, fixtures, and apiServices; if adding helpers/page objects, place them in the right scope (plugin vs solution vs @kbn/scout) and register via fixtures.eslint-disable, or swapping a flagged pattern for a hack that hides it). Sanctioned escape hatches (e.g. dispatchEvent('click') for a documented app bug) are fine only with an explicit documented justification. Don't re-flag plain lint violations — CI already gates those.apiClient for the endpoint under test; apiServices/kbnClient for setup/teardown only; correct auth + common headers.expect), test flow (test.step), and page-object method calls. Short inline locator calls for simple one-off assertions (e.g. a single label or nav-link check) are acceptable. Flag raw locators when the interaction is complex enough to benefit from abstraction or is duplicated across specs. Extract all locators as readonly properties in the constructor; no inline locator creation inside methods.afterAll/afterEach; defensive cleanup in beforeAll for failed-run leftovers; scoutSpace.savedObjects.cleanStandardList() as catch-all after domain-specific cleanup; no reliance on file ordering or shared mutable state.admin unless required); space-aware behavior covered or explicitly out of scope.waitForTimeout() and time-based assertions/retries; rely on auto-waiting + explicit readiness signals. Some locators are restricted by @kbn/eslint/scout_no_locators (e.g. globalLoadingIndicator).global.teardown.ts is present): cleanup must use esClient/kbnClient/apiServices. esArchiver isn't on the teardown fixture surface — Scout intentionally never exposed archive-unloading (slow and unnecessary; leftover indexes don't break tests with idempotent loadIfNeeded). Flag teardowns that try to use esArchiver at all, that load new data (teardown is for state reset only), or that duplicate work belonging in afterAll/per-test cleanup.Do not review or comment on:
.meta manifest files (e.g., **/.meta/**/*.json): these are auto-generated for CI test planning and lane distribution. No manual regeneration is needed.Use these definitions when assigning severity:
When in doubt, prefer a lower severity. Optimization suggestions (efficiency improvements) should be minor or nit, not major.
Flag only major changes or drops — coverage that genuinely weakens. Skip benign deltas.
test/functional/**, loadTestFile(), FTR configs) alongside new/changed Scout specs.blocker unless explicitly de-scoped.admin vs viewer), spaces behavior, and permission realismignoreErrors, automatic retries)loadTestFile()).These EUI/Kibana component behaviours are non-obvious and cannot be inferred from documentation alone.
QueryStringInput: fill() races with React prop sync; use pressSequentially() instead.EuiBasicTable empty state: always renders a phantom "no items found" row — assert toContainText('No items found'), never toHaveCount(0).span:has([data-test-subj="..."]) wrapper, not the button itself..euiTableRow, .euiToolTipAnchor, etc.): internal to EUI, change between versions — use data-test-subj or ARIA roles.dispatchEvent('click') over { force: true }; document the bug location in a comment.page.components.*): prefer the published @elastic/eui-test-helpers (via page.components.*) over raw selectors or the old EuiXxxWrappers; flag self-extension (local subclasses / one-off helper methods) — missing capabilities belong in the published helper via DevEx. See the scout-ui-testing skill for details.This skill does not prescribe an output format. The caller decides how findings are reported:
OUTPUT.md.Offer to generate the updated code, fully incorporating the suggested improvements and resolving any parity gaps.