test(regression): promote run-flow.spec.ts to @stable - #268
Conversation
Refactor for CI hygiene and weekly-run stability: - Add @stable + @regression tags. - Trim 4x timeout: 100000ms (100s) to 30000ms (30s) — project standard. - Fix L80 no-op assertion: the existing `await page.getByText("New Flow").isVisible();` discarded the return value (always passed regardless of visibility). Replaced with `await expect(...).toBeVisible({ timeout })`. - Wrap test body in try/finally with API-based cleanup of the 2 flows the test creates (previously orphaned every run). Uses getAuthToken + DELETE /api/v1/flows/{id} for speed and to avoid cascading UI failures during cleanup. - ESLint --fix promoted the final assertion from `expect(await ...inputValue()).toBe(...)` to the auto-waiting `await expect(...).toHaveValue(...)`. QA-CHECKLIST L589: correct stale path (core/features/run-flow.spec.ts -> flow-functionality/run-flow.spec.ts) and inaccurate description ("Execute flow via Run button" -> "Run Flow component executes another flow"). The spec tests the Run Flow component on canvas, not the canvas Run button. This spec is 100% non-redundant — no other test covers the Run Flow component on canvas. api-run-flow.spec.ts (@stable) covers the pure API path; this covers the UI surface that uses internal mechanisms.
There was a problem hiding this comment.
Pull request overview
Promotes the run-flow.spec.ts regression test to @stable after fixing a no-op visibility assertion, trimming oversized timeouts, adding API-based cleanup of the two flows it creates, and correcting a stale entry in QA-CHECKLIST.
Changes:
- Adds
@stable/@regressiontags and tightens four 100s timeouts down to 30s; replaces.isVisible()no-op withawait expect(...).toBeVisible()and switches the final input assertion to auto-waitingtoHaveValue. - Wraps the test body in
try/finallyand best-effort deletes the 2 most recently created flows viagetAuthToken+DELETE /api/v1/flows/{id}. - Updates QA-CHECKLIST entry 12.6 to mark the spec stable and corrects its path/description.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/tests-automations/regression/flow-functionality/run-flow.spec.ts | Promote to @stable, trim timeouts, fix no-op assertion, add API cleanup in finally. |
| QA-CHECKLIST.md | Mark Run Flow component scenario as [x] and fix stale path/description. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const listRes = await request.get("/api/v1/flows/", { headers }); | ||
| if (listRes.ok()) { | ||
| const body = await listRes.json(); | ||
| const items = (Array.isArray(body) ? body : body?.items ?? []).slice( |
There was a problem hiding this comment.
Valid — fixed in b89103e. You're right that the object-form fallback used the wrong key (body?.items vs the actual body.flows shape established by tests/helpers/flows/clean-all-flows.ts:33). The bug never bit because Langflow currently returns the array form on this endpoint, but a future change to the response shape would have silently skipped cleanup.
The whole listing call is gone now — replaced with the same ID-scoping pattern landed in PR #267 (bulk-actions): the test captures the flow ID from page.url() after each blank-flow.click() and the finally block iterates only those captured IDs. No more list-then-slice; no more object-form ambiguity.
| const listRes = await request.get("/api/v1/flows/", { headers }); | ||
| if (listRes.ok()) { | ||
| const body = await listRes.json(); | ||
| const items = (Array.isArray(body) ? body : body?.items ?? []).slice( | ||
| 0, | ||
| 2, | ||
| ); | ||
| for (const f of items) { | ||
| await request.delete(`/api/v1/flows/${f.id}`, { headers }); | ||
| } |
There was a problem hiding this comment.
Valid — fixed in b89103e. Same root issue as comment 1 (positional slice(0, 2) over the listing). Two concrete risks the previous code had:
- The list did not use
remove_example_flows=true, so any example/starter flow that the API returns ahead of user-created flows would have been deleted. - Under
fullyParallel: true, a sibling worker could have created flows that ended up in the top-2 positions during the window betweenrequest.get(...)andrequest.delete(...), taking collateral damage.
Fix is the same pattern from PR #267:
- Capture
createdFlowIds: string[]populated bypage.waitForURL(/\/flow\/[0-9a-f-]+/i)+ regex extraction after eachblank-flow.click(). finallyblock iterates only those IDs and callsDELETE /api/v1/flows/{id}for each. 404s for IDs the test already deleted on the happy path are silenced.
Validated against the live API: 182 flows before the run, 182 after — the 2 captured IDs were deleted, nothing else was touched.
Document the Run Flow component test with the post-refactor behavior: - toHaveValue auto-waiting assertion (replaces toBe(inputValue)) - try/finally API cleanup of the 2 created flows via getAuthToken
Addresses Copilot review on PR #268. Two bugs in the cleanup, same root: positional slice over an incorrectly-normalised list. 1. Comment 1 (L143): the object-form fallback was `body?.items` but `/api/v1/flows/` returns the array under `flows` (see `tests/helpers/flows/clean-all-flows.ts:33`). The object-form branch would have silently produced an empty list and skipped the entire cleanup. Bug never bit because Langflow currently returns the array form, but Copilot is right that the fallback was wrong. 2. Comment 2 (L149): `slice(0, 2)` over the listing trusted that the two most-recently-created flows were the two this test built. If the listing returns example/starter flows first (a real default when `remove_example_flows=true` is not set), or if a sibling worker created flows under `fullyParallel`, this would delete unrelated flows. Fix is the same pattern landed in PR #267 (bulk-actions): capture the flow IDs as they are created via `page.url()` after each `blank-flow.click()`, then `DELETE /api/v1/flows/{id}` for each captured ID in the `finally` block. The listing call is gone entirely, which also removes the `body?.items` vs `body?.flows` ambiguity. 7-step validation pipeline: - typecheck clean - lint: 0 errors, 10 pre-existing warnings (1 fewer than before) - run --workers=1 --retries=0: PASS (17.9s) - Force-fail on the final `toHaveValue(...)`: failed at L148 with received "THIS IS A TEST FOR RUN FLOW COMPONENT", reverted, repassed - Trace coherent, zero backend errors - Cleanup validated against the live API: 182 flows before run, 182 after (the 2 created flows were deleted)
Summary
Promotes `tests/tests-automations/regression/flow-functionality/run-flow.spec.ts` to `@stable` after fixing a no-op assertion, trimming excessive timeouts, adding API-based cleanup, and correcting a stale QA-CHECKLIST entry.
Why this spec is non-redundant
This is the only test covering the Run Flow component on canvas — a Langflow component that invokes another flow from within a flow.
`api-run-flow.spec.ts` (`@stable`) covers `POST /api/v1/run/{flow_id}` directly via API — different surface. No other UI spec covers flow chaining via the Run Flow component.
Changes
Test plan
Known limitation (out of scope)
The cleanup deletes the 2 most-recently-created flows from the listing, assuming reverse-chronological ordering. If a prior orphan from a different test appears newer than the 2 created here, cleanup could delete the wrong flow. Acceptable trade-off — cleanup is best-effort hygiene, and Langflow's listing API correctly orders by recency.