Expand FTR - Scout migration SKILLs (tags, tsconfig, saml API auth)#259520
Merged
dmlemeshko merged 10 commits intoelastic:mainfrom Apr 7, 2026
Merged
Expand FTR - Scout migration SKILLs (tags, tsconfig, saml API auth)#259520dmlemeshko merged 10 commits intoelastic:mainfrom
dmlemeshko merged 10 commits intoelastic:mainfrom
Conversation
- scout-api-testing: clarify tag choice for solution vs deploymentAgnostic - scout-create-scaffold: Pattern A/B TypeScript layout, link to migrate-from-ftr - scout-migrate-from-ftr: deployment-agnostic tag guidance, cookieHeader vs API key, synthtrace API fixtures, Pattern A/B typecheck pitfalls Split from onboarding Scout API migration PR for focused review. Made-with: Cursor
dmlemeshko
added
release_note:skip
dmlemeshko
changed the title
csr
reviewed
Mar 31, 2026
| - `x-pack/solutions/security/**` -> `@kbn/scout-security` | ||
| - Prefer a single top-level `apiTest.describe(...)` per file and avoid nested `describe` blocks; multiple top-level `describe`s are supported, but files get hard to read quickly. | ||
| - Tags: add `{ tag: ... }` on the suite (or individual tests) so CI/discovery can select the right test target (for example `tags.deploymentAgnostic` or `[...tags.stateful.classic]`). Unlike UI tests, API tests don’t currently validate tags at runtime. | ||
| - Tags: add `{ tag: ... }` on the suite (or individual tests) so CI/discovery can select the right test target. For **solution** modules, prefer explicit targets (e.g. `[...tags.stateful.classic, ...tags.serverless.observability.complete]` in Observability); reserve `tags.deploymentAgnostic` mainly for **platform** specs that truly need every deployment-agnostic target (see `scout-migrate-from-ftr`). Unlike UI tests, API tests don’t currently validate tags at runtime. |
Member
There was a problem hiding this comment.
Great point on the deployment agnostic tag, I've updated the docs to reflect this info too: #260229
(will continue the review as soon I find the time)
csr
reviewed
Mar 31, 2026
Comment on lines
+175
to
+191
| #### Where Scout tests are typechecked (choose one) | ||
|
|
||
| **Pattern A — plugin `tsconfig` includes Scout (recommended when tests import `server/` or `common/`)** | ||
|
|
||
| - Extend the plugin **`tsconfig.json`** with e.g. **`test/scout/**/*`** (or **`test/**/*`** like `discover_enhanced`). | ||
| - Add solution Scout packages to **`kbn_references`** on that same file (e.g. **`@kbn/scout-oblt`**; for API + synthtrace also **`@kbn/scout-synthtrace`**, **`@kbn/synthtrace-client`**). Platform-only tests use **`@kbn/scout`** instead of `scouting-oblt` where appropriate. | ||
| - **Do not** add `test/scout/ui/tsconfig.json` or `test/scout/api/tsconfig.json`; `yarn kbn bootstrap` should not list separate Scout tsconfigs for this plugin. | ||
| - **Relative imports** from specs/fixtures into **`server/`** or **`common/`** are valid: tests share one TS program with the plugin (same `rootDir` / `include` story as FTR-style colocated tests). | ||
|
|
||
| **Pattern B — dedicated `test/scout/{ ui, api }/tsconfig.json`** | ||
|
|
||
| - Keeps the **plugin** `type_check` graph smaller; matches many existing modules (e.g. SLO, `data_views` API, infra Scout UI). | ||
| - The Scout folder is its **own** composite project with `rootDir` under `test/scout/...` only. | ||
| - **Do not** use relative imports that reach **`../../../../server/...`** or **`public/...`** from those specs—the importer’s project will try to own `server/**/*.ts` and CI **`check_types`** will explode with **`TS6059` / `TS6307`** (see `tsc --listFilesOnly`). | ||
| - **Do instead:** constants in **`fixtures/constants.ts`** (with a “must match …” comment), **`@kbn/*`** imports covered by that project’s `kbn_references`, or **`common/`** modules that typecheck inside the Scout project. | ||
|
|
||
| **Choosing:** Prefer **Pattern A** when migrating FTR tests that already imported registration constants or server helpers. Prefer **Pattern B** when you want minimal plugin compile cost and can keep imports boundary-safe. |
Member
There was a problem hiding this comment.
If this information is already in the scout-create-scaffold skill then I don't think it makes sense to duplicate here too: it's a lot of duplicated tokens. Unless you saw the agent got it wrong?
csr
reviewed
Mar 31, 2026
Comment on lines
+121
to
+145
| #### Scout API auth (`cookieHeader` vs API key) | ||
|
|
||
| FTR often used `roleScopedSupertest` with `useCookieHeader: true` and `withInternalHeaders` for **internal** routes. In Scout API tests, map that to **`samlAuth`** and merge **`cookieHeader`** with your common headers (`kbn-xsrf`, `x-elastic-internal-origin`, etc.) on `apiClient` requests. | ||
|
|
||
| ```ts | ||
| const { cookieHeader } = await samlAuth.asInteractiveUser('admin'); | ||
| const adminInteractiveHeaders = { | ||
| ...MY_COMMON_HEADERS, | ||
| ...cookieHeader, | ||
| }; | ||
| await apiClient.post('internal/my_plugin/flow', { | ||
| headers: adminInteractiveHeaders, | ||
| responseType: 'json', | ||
| }); | ||
| ``` | ||
|
|
||
| **When to prefer `cookieHeader` (interactive user) over `requestAuth.getApiKey(...)`** | ||
|
|
||
| - Handlers that call **`core.security.authc.apiKeys.create`** (or otherwise create **nested** API keys for the current user) often **fail with HTTP 500** if the incoming request is authenticated with an **API key**. Use **`samlAuth.asInteractiveUser('admin')`** (or the same role FTR used with cookies) for those routes—see observability onboarding `POST internal/observability_onboarding/flow` migration (`89c171c3851c1c6f70f55749b5477b1f0adfffb8`). | ||
| - **Negative / least-privilege tests** that in FTR used a **custom role + cookie** (e.g. empty `kibana: []` privileges) and expect **404** from scoped saved-object access: use **`await samlAuth.asInteractiveUser(customRoleDescriptor)`** and **`cookieHeader`** on the request. **`requestAuth.getApiKeyForCustomRole(...)`** can still resolve **200** for the same role shape because API-key privilege resolution differs from an interactive session—match FTR with cookies. | ||
|
|
||
| **When API keys are still appropriate** | ||
|
|
||
| - Public `api/*` routes, or internal routes that **do not** create nested keys and were explicitly tested with API key in FTR (e.g. some “terminal” step reporters). | ||
| - Use `requestAuth.getApiKey('admin')` / `getApiKeyForCustomRole` per `scout-api-testing`; combine with `cookieHeader` in the **same** spec when different calls need different auth modes. |
Member
There was a problem hiding this comment.
This information should partially already be in this doc: https://github.com/elastic/kibana/blob/main/docs/extend/scout/api-auth.md (live version). I suggest we simply reference that doc (maybe put it in a Reference section). We could mention FTR specific details and then link to the existing doc. What do you think?
csr
requested changes
Mar 31, 2026
csr
reviewed
Mar 31, 2026
Comment on lines
+66
to
+75
| #### Tags when the FTR suite was “deployment agnostic” | ||
|
|
||
| FTR **deployment-agnostic** configs often load the same files under both stateful and serverless. In Scout, **do not assume** `tags.deploymentAgnostic` is the right default for every migrated spec—it selects **many** targets (including stateful/search/security and serverless/search/security, etc.). | ||
|
|
||
| - **Tests colocated under a solution plugin or package** (`x-pack/solutions/observability|security|search/...`, using `@kbn/scout-oblt`, `@kbn/scout-security`, or `@kbn/scout-search`): Prefer **explicit solution targets** instead of `tags.deploymentAgnostic`, so CI only runs where that solution is present and supported. Typical pattern (adjust domain to your solution): | ||
| - Observability: `{ tag: [...tags.stateful.classic, ...tags.serverless.observability.complete] }` (and add `tags.serverless.observability.logs_essentials` only if the feature is meant to run on that project type). | ||
| - Security / Search: use the corresponding `tags.serverless.*` and `tags.stateful.*` entries from the same Scout package—**match sibling specs** in that module. | ||
| - **Tests colocated under platform** (`src/platform/**`, `x-pack/platform/**`, `@kbn/scout` only): If the original intent was “run everywhere the deployment-agnostic FTR job runs”, **`tags.deploymentAgnostic`** is still appropriate. | ||
|
|
||
| API and UI specs should both carry tags that match the intended `run-tests` / CI targets; see step 9. |
Member
There was a problem hiding this comment.
This information is now in the deployment tags document (docs/extend/scout/deployment-tags.md). Do you think we can simply reference to that doc? Maybe something like: "to understand which tag to assign refer to x doc".
csr
reviewed
Mar 31, 2026
Member
csr
left a comment
csr
left a comment
There was a problem hiding this comment.
I added a couple of comments to hopefully rely on our docs more and make the skills thinner. I hope this won't be to the detriment of skill accuracy / efficiency, but I'd like to think that won't be the case. Thanks for your patience while I took the time to review this.
dmlemeshko
commented
Apr 7, 2026
Comment on lines
+86
to
+88
| #### Combine duplicate stateful / serverless FTR tests | ||
|
|
||
| FTR often has **separate but near-identical** test files under `test/*api_integration*/` (stateful) and `test/serverless/` (or similar directories). Before migrating each file individually, compare them: if the test flow is identical or almost identical, **combine into a single Scout spec** with tags covering both deployment targets (e.g. `[...tags.stateful.classic, ...tags.serverless.observability.complete]`). Extract any deployment-specific differences into conditional helpers or small branching within the spec. Only keep separate specs when the flows genuinely diverge. |
Contributor
Author
There was a problem hiding this comment.
New: combining stateful + serverless might be tricky, adding explicit details
dmlemeshko
commented
Apr 7, 2026
| - `beforeEach/afterEach` -> `test.beforeEach/test.afterEach`. | ||
| - Keep **one suite per file** and a flat hierarchy (avoid nested `describe`; use `test.step()` inside a test for structure). | ||
| - If a single FTR file contains multiple top-level `describe` blocks, split into multiple Scout specs (one describe per file). | ||
| - **Nested `describe` blocks**: if the FTR file has nested describes, prefer splitting into separate Scout spec files. However, if the file is small and the nested describes are lightweight, flatten them into a single `test.describe` with individual `test(...)` blocks using `test.step(...)` for sub-structure instead of creating many tiny spec files. |
Contributor
Author
There was a problem hiding this comment.
New: I noticed few issues when migrated Scout API test had nested describe blocks, let's clarify how to handle it.
dmlemeshko
commented
Apr 7, 2026
| ### 4) Replace FTR dependencies | ||
|
|
||
| - Replace `supertest` calls with Scout `apiClient` (endpoint under test) + `requestAuth`/`samlAuth` (auth). | ||
| - Replace `supertest` calls with Scout `apiClient` (endpoint under test) + `requestAuth`/`samlAuth` (auth). FTR stateful tests often use `supertest` with an implicit **admin** role—don't carry that over blindly. Research whether a lower default role like `editor` or `viewer` is sufficient; comparing the roles used in the **serverless** version of the same test (if one exists) is a good starting point. |
Contributor
Author
There was a problem hiding this comment.
New: stateful tests uses supertest and we shouldn't migrate it as-is without review
dmlemeshko
commented
Apr 7, 2026
| - Replace `supertest` calls with Scout `apiClient` (endpoint under test) + `requestAuth`/`samlAuth` (auth). FTR stateful tests often use `supertest` with an implicit **admin** role—don't carry that over blindly. Research whether a lower default role like `editor` or `viewer` is sufficient; comparing the roles used in the **serverless** version of the same test (if one exists) is a good starting point. | ||
| - Replace other FTR services with Scout fixtures (`pageObjects`, `browserAuth`, `apiServices`, `kbnClient`, `esArchiver`). | ||
| - Use `apiServices`/`kbnClient` for setup/teardown and verifying side effects. | ||
| - **Audit FTR before/after hooks carefully**—don't copy them verbatim. Review every call in `before`/`beforeEach`/`after`/`afterEach` and verify it is still correct for Scout: replace FTR-specific APIs with their Scout equivalents, remove unnecessary calls (e.g. FTR service initialization that Scout fixtures handle automatically), and add any missing setup or cleanup that the FTR suite neglected. Ensure every resource created in `beforeAll`/`beforeEach` has matching cleanup in `afterAll`/`afterEach`—FTR suites frequently lack proper teardown. Place `kbnClient.savedObjects.cleanStandardList()` (or `scoutSpace.savedObjects.cleanStandardList()`) in **`afterAll`**, not `beforeAll`; `beforeAll` cleanup masks missing teardown and hides leaked state from previous runs. |
Contributor
Author
There was a problem hiding this comment.
New: hooks is where runtime gets lost, review if actions are needed and don't do cleanup in before hook, but in after
csr
approved these changes
Apr 7, 2026
Member
csr
left a comment
csr
left a comment
There was a problem hiding this comment.
I love these recent changes, LGTM!
kelvtanv
pushed a commit
to kelvtanv/kibana
that referenced
this pull request
Apr 7, 2026
…lastic#259520) ## Summary: During elastic#259436 I faced few challenges + broken tests in Scout, hopefully SKILLs update make migration more robust File | Change (high level) -- | -- scout-api-testing/SKILL.md | Solution vs deploymentAgnostic tagging for API tests scout-create-scaffold/SKILL.md | Related skill link, Pattern A/B TS layout, post-generate steps scout-migrate-from-ftr/SKILL.md | DA tags, cookieHeader vs API key, synthtrace API, Pattern A/B, pitfalls
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
3 participants
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:
During #259436 I faced few challenges + broken tests in Scout, hopefully SKILLs update make migration more robust