Skip to content

Revert "test(ui): run the live Playwright suite in CI against the built UI image (#2874)" #2910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
EItanya merged 1 commit into from
Sep 23, 2026
Merged

Revert "test(ui): run the live Playwright suite in CI against the built UI image (#2874)" #2910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 4 additions & 124 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ jobs:
VERSION: v0.0.1-test
SUBSTRATE_VERSION: 0.2.0-beta5
runs-on: blacksmith-4vcpu-ubuntu-2404
# 50: main's own 40, kept as headroom. The job itself takes around 10 minutes,
# UI image and browser suite included, on a warm buildx cache.
timeout-minutes: 50
timeout-minutes: 40
steps:
- name: Checkout repository
uses: actions/checkout@v6
Expand Down Expand Up @@ -120,6 +118,7 @@ jobs:
KMCP_ENABLED: "false"
KAGENT_HELM_EXTRA_ARGS: >-
--cleanup-on-fail=false
--set ui.replicas=0
--set kagent-tools.enabled=false
--set grafana-mcp.enabled=false
--set controller.substrate.enabled=true
Expand All @@ -137,21 +136,11 @@ jobs:
BUILDX_BUILDER_NAME=$(docker buildx inspect | awk '$1 == "Name:" { print $2; exit }')
test -n "$BUILDX_BUILDER_NAME"
export BUILDX_BUILDER_NAME
# Before the fan-out. Each `build-*` runs it again — it is not passed `-o` the
# way `proto-generate` is below — which is harmless only because the target
# documents itself as safe to run concurrently.
make buildx-create
# Likewise once, and for a sharper reason: `build-ui` and `build-golang-adk` both
# depend on it, and `buf generate` writes `ui/src/generated` as well as
# `go/api/gen` — so in parallel one make would write into the directory the
# other is tarring as a build context. `-o proto-generate` below says it is done.
make proto-generate
# `ui` is here so the browser suite below runs against the image an operator
# gets: nginx, the SPA fallback, and the env-config.js rendered at pod start.
printf '%s\n' controller ui golang-adk claude-harness codex-harness byo-a2a | xargs -P5 -n1 bash -c '
printf '%s\n' controller golang-adk claude-harness codex-harness byo-a2a | xargs -P4 -n1 bash -c '
image="$1"
DOCKER_BUILD_ARGS="--platform=linux/amd64 --push" \
make -o proto-generate GIT_COMMIT=e2e BUILD_DATE=1970-01-01 "build-${image}"
make GIT_COMMIT=e2e BUILD_DATE=1970-01-01 "build-${image}"
' _
make helm-install-provider
kubectl rollout status deployment/kagent-controller -n kagent --timeout=120s
Expand Down Expand Up @@ -192,111 +181,6 @@ jobs:
export KAGENT_E2E_OTLP_LISTEN_ADDRESS=:14317
go test -v github.com/kagent-dev/kagent/go/core/test/e2e -failfast -shuffle=on -parallel 4 -count=1

# The `smoke` template is applied part-way through the step above, after four
# digest lookups. The browser journeys below need an agent to schedule and a
# template to list, so a step that died before that apply would fail them for a
# reason that has nothing to do with the UI — three retries each, against an empty
# cluster. `continue-on-error`, because a missing template means the Go step has
# already failed the job and this only decides whether the UI steps are worth
# running: `outcome` still reports the failure the gate below reads.
- name: Check the cluster has the smoke agent
id: smoke-agent
if: ${{ !cancelled() && steps.install-kagent.outcome == 'success' }}
continue-on-error: true
# Retried, because everything below is gated on this one command. Asked once, a
# transient API-server or kubeconfig hiccup skips the whole browser lane and the
# job still reports green — which is the shape of #2638, a suite that measured
# nothing while every check passed.
run: |
for attempt in $(seq 1 10); do
if kubectl get agenttemplate smoke -n kagent; then exit 0; fi
echo "attempt ${attempt}: not there yet"
sleep 6
done
kubectl get agenttemplate -A || true
exit 1

# And when it is genuinely absent, said out loud on the run. A skipped lane and a
# passing one look identical in the checks list otherwise.
- name: Say so if the UI suite was skipped
if: ${{ !cancelled() && steps.smoke-agent.outcome != 'success' }}
run: |
echo "::warning title=UI live suite skipped::No smoke agent on the cluster, so the live Playwright lane did not run."

# The UI, against this same cluster and the image built above. `ui-tests` runs on
# the in-browser mock, so a backend contract change cannot fail it — which is how
# #2638 emptied three Substrate fields with every check green.
#
# After the Go tests, not beside them: both create and delete resources in the same
# namespace, and these journeys read the lists back.
#
# Gated on the cluster being usable rather than on `success()`, so one failed Go
# test under `-failfast` does not also cost every UI signal. No cluster or no
# agent, no run.
- name: Setup Node.js
if: ${{ !cancelled() && steps.smoke-agent.outcome == 'success' }}
uses: actions/setup-node@v7
with:
node-version-file: ui/.nvmrc

# Before the cache step, as in `ui-tests`: package.json pins Yarn 4, and the
# runner's own shim cannot read this lock file.
- name: Enable Corepack
if: ${{ !cancelled() && steps.smoke-agent.outcome == 'success' }}
run: corepack enable

- name: Cache Yarn downloads
if: ${{ !cancelled() && steps.smoke-agent.outcome == 'success' }}
uses: actions/cache@v6
with:
path: ui/.yarn/cache
key: yarn-${{ runner.os }}-${{ hashFiles('ui/yarn.lock') }}
restore-keys: yarn-${{ runner.os }}-

- name: Prepare the live browser suite
if: ${{ !cancelled() && steps.smoke-agent.outcome == 'success' }}
working-directory: ./ui
# Chromium only: the live suite declares one project. The mock suite's second
# engine is there to disagree about layout, which no backend contract rests on.
run: |
yarn install --immutable
yarn playwright install --with-deps chromium

- name: Run live browser tests
if: ${{ !cancelled() && steps.smoke-agent.outcome == 'success' }}
working-directory: ./ui
run: |
kubectl -n kagent rollout status deploy/kagent-ui --timeout=5m
# Already a LoadBalancer on a MetalLB cluster, so this is the address the
# controller e2e step takes — not a port-forward to keep alive for the run.
UI_IP="$(kubectl get svc -n kagent kagent-ui -o jsonpath='{.status.loadBalancer.ingress[0].ip}')"
test -n "$UI_IP"
export UI_LOOP_LIVE_URL="http://${UI_IP}:8080"
echo "UI_LOOP_LIVE_URL: $UI_LOOP_LIVE_URL"
# `globalSetup` refuses the run if that address answers with fixtures, or is a
# dev server rather than the built image.
yarn test:pw:live

- name: Upload live browser report
# Any finished run, not just a failed one: CI retries twice, so a spec that
# fails and then passes leaves a green job and no trace of the failure — which
# is the run whose trace is worth the most. A clean run uploads the HTML report
# and nothing else, since `test-results` only has content when something failed.
if: ${{ !cancelled() && steps.install-kagent.outcome == 'success' }}
uses: actions/upload-artifact@v5
with:
name: ui-live-playwright-report
# The trace is the only record of what the cluster answered — there is no fixed
# fixture to re-read afterwards, as there would be for the mock suite. The HTML
# report is what links one to the other, and it exists because the config asks
# for `html` alongside `github`; `github` on its own writes annotations and no
# files, which is how this path came to be uploaded empty.
path: |
ui/playwright-report
ui/test-results
retention-days: 7
if-no-files-found: ignore

- name: fail print info
if: failure()
run: |
Expand All @@ -309,10 +193,6 @@ jobs:
kubectl get harnesses,agenttemplates -n kagent
echo "::error::Kubectl logs -n kagent deployment/kagent-controller"
kubectl logs -n kagent deployment/kagent-controller
# nginx logs every proxied request, so a browser failure that was really a
# backend failure says so here, as does an init.sh that rejected a value.
echo "::error::Kubectl logs -n kagent deployment/kagent-ui"
kubectl logs -n kagent deployment/kagent-ui --tail=200 || true
kubectl get pods,actortemplates,workerpools -A
echo "::error::Substrate logs"
kubectl logs -n ate-system -l app --all-containers --prefix --tail=200 || true
Expand Down
7 changes: 1 addition & 6 deletions ui/eslint.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,9 @@ export default tseslint.config(
*
* `playwright/README.md` states these; a convention nothing checks is one that
* regrows as an exception.
*
* Every folder that holds specs, not only `tests/`: `shared/` runs in every project
* and `live/` is the only suite that talks to a cluster, so a spec there trusting its
* own green matters more rather than less. Scoping this to `tests/` alone left both
* outside the guard from the day they were added.
*/
{
files: ["playwright/{tests,shared,live}/**/*.spec.ts"],
files: ["playwright/tests/**/*.spec.ts"],
rules: {
"no-restricted-imports": [
"error",
Expand Down
128 changes: 33 additions & 95 deletions ui/playwright.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,6 @@ const EXTENSION_BASE_URL = `http://localhost:${EXTENSION_PORT}`;
/** Specs that need an extension installed opt in by filename. */
const EXTENSION_SPECS = /\.withExtension\.spec\.ts$/;

/**
* Specs that run in both suites, claimed by every project below.
*
* Only what holds on either backend — no `?mock=` scenario, no fixture by name. That
* narrowness is the point and the risk: assertions true of both are the weakest ones,
* so this folder stays a smoke sweep rather than growing lifecycles.
* `conventions.test.ts` fails a spec here that reaches for a scenario.
*
* A glob, not a regex: Playwright matches a `testMatch` regex against the *absolute*
* path, so `/shared\//` also matches every spec in a checkout that happens to live
* under a directory of that name. A glob resolves against `testDir`, which is the
* thing actually meant here.
*/
const SHARED_SPECS = "shared/**/*.spec.ts";

/**
* The suite is the acceptance bar, so what it runs against cannot depend on the
* shell it was started from: both servers are pinned to the in-browser mock
Expand Down Expand Up @@ -91,71 +76,33 @@ const LIVE = process.env.UI_LOOP_LIVE === "true";
* server is about to want, or vice versa.
*/
const LIVE_PORT = Number(process.env.UI_LOOP_LIVE_PORT ?? 8301);

/**
* An app already running — in CI, the `kagent-ui` service — tested instead of one
* this config starts. That is the deployed image, so nginx, the SPA fallback and the
* `env-config.js` rendered at pod start are under test rather than approximated by
* Vite. A developer still gets the dev server by default; the alternative is building
* an image to run a test.
*
* Empty counts as absent, and the `||` is what makes that one question rather than
* three. Three things below ask it — what `baseURL` is, whether `globalSetup` checks
* for a deployed image, whether a dev server is started — and they had asked it three
* ways: a `??`, a `!== undefined` and a truthiness test. An exported-but-empty
* variable, which is what a `kubectl get svc` that found nothing leaves behind, then
* pointed the run at `""`, checked that for nginx, *and* started a dev server nothing
* would ever visit.
*/
const LIVE_EXTERNAL_URL = process.env.UI_LOOP_LIVE_URL || undefined;

const LIVE_BASE_URL = LIVE_EXTERNAL_URL ?? `http://localhost:${LIVE_PORT}`;
const LIVE_BASE_URL = `http://localhost:${LIVE_PORT}`;

/** Read by `playwright/globalSetup.ts` to decide what to verify about a server. */
export const LIVE_PROJECT = "chromium-live";

/**
* Whether this live run is against a deployed app rather than a dev server, which
* `globalSetup` checks one more thing for — see `verifyLiveWiring`.
*/
export const LIVE_IS_DEPLOYED = LIVE_EXTERNAL_URL !== undefined;

/**
* How the *dev server* is configured for a live run; a deployment is configured by its
* chart instead. It proxies `/api` the way nginx does in a cluster, so the app uses the
* same relative URLs either way — standing in for nginx rather than being it, which is
* the gap `UI_LOOP_LIVE_URL` closes. `VITE_API_MODE` is pinned at build time as well as
* at runtime, being the one thing an inherited `.env` cannot override.
* A live run reaches the backend through Vite's proxy, exactly as a deployed
* build reaches it through nginx — so the app uses the same relative URLs either
* way and this mode tests the addressing a real deployment uses.
*
* `VITE_API_MODE` is pinned as well as the runtime flag: the build-time pin is
* the one thing an inherited `.env` cannot override, and a live suite that
* silently answered from fixtures would be worse than a red one.
*/
const LIVE_APP = { VITE_API_MODE: "live", ENABLE_MOCK_UI: "false" };

/** How the live server is started. Unused when `UI_LOOP_LIVE_URL` names one already. */
const LIVE_COMMAND = `yarn dev --port ${LIVE_PORT}`;

/**
* The servers a live run starts, which is none when it was handed one: there is no
* process to own — the app is a pod. `globalSetup` checks the address serves the app.
* How the live server is started.
*
* Named for the same reason the three env pins above are: a branch whose backend
* needs more than a dev server — a credential minted per run, a port-forward
* probed before Vite starts — replaces this line rather than the block below.
*/
const LIVE_WEB_SERVERS = LIVE_EXTERNAL_URL
? []
: [
{
command: LIVE_COMMAND,
url: LIVE_BASE_URL,
reuseExistingServer: false,
timeout: 120_000,
// Whatever starts the live server is the most useful output a failed
// live run has — something that cannot reach the backend says so there,
// and Playwright discards a web server's stdout unless asked to pass it
// through.
stdout: "pipe" as const,
stderr: "pipe" as const,
env: LIVE_APP,
},
];
const LIVE_COMMAND = `yarn dev --port ${LIVE_PORT}`;

export default defineConfig({
testDir: "./playwright",
testDir: "./playwright/tests",
// Both servers have to be rendering, not merely listening, before any test
// navigates — see the file for what goes wrong otherwise.
globalSetup: "./playwright/globalSetup.ts",
Expand Down Expand Up @@ -183,15 +130,7 @@ export default defineConfig({
* of the machine than the local runs that provoked it.
*/
workers: process.env.CI ? "50%" : undefined,
/*
* Both in CI: `github` writes the annotations that put a failure on the diff, and it
* writes no files at all — so the report CI uploads as an artifact has to come from
* somewhere, and for a live failure the trace is the only account of what the cluster
* answered.
*/
reporter: process.env.CI
? [["github"], ["html", { open: "never" }]]
: [["list"]],
reporter: process.env.CI ? "github" : "list",
/*
* A real backend behind a port-forward answers in tens of seconds where the
* in-browser mock answers in milliseconds, so the defaults that suit the mock
Expand All @@ -204,20 +143,7 @@ export default defineConfig({
* and a mock-backed suite that needs more than thirty seconds for one test is saying
* something is stuck, which is worth hearing rather than absorbing.
*/
...(LIVE
? {
timeout: 120_000,
expect: { timeout: 30_000 },
/*
* One at a time. Every mock test owns a backend in its own page's memory; these
* share a cluster, so a spec creating a resource while another counts them is a
* failure with no defect behind it. A dozen tests, and one cluster to run them
* against — it costs little.
*/
workers: 1,
fullyParallel: false,
}
: {}),
...(LIVE ? { timeout: 120_000, expect: { timeout: 30_000 } } : {}),
use: {
trace: "on-first-retry",
screenshot: "only-on-failure",
Expand All @@ -227,7 +153,7 @@ export default defineConfig({
? [
{
name: LIVE_PROJECT,
testMatch: ["live/**/*.spec.ts", SHARED_SPECS],
testDir: "./playwright/live",
use: {
...devices["Desktop Chrome"],
baseURL: LIVE_BASE_URL,
Expand All @@ -241,7 +167,6 @@ export default defineConfig({
: [
{
name: "chromium",
testMatch: ["tests/**/*.spec.ts", SHARED_SPECS],
testIgnore: EXTENSION_SPECS,
use: { ...devices["Desktop Chrome"], baseURL: BASE_URL },
},
Expand All @@ -257,7 +182,6 @@ export default defineConfig({
// The extension split below is a build-time difference, not a browser one,
// so it stays on one engine rather than doubling for no new signal.
name: "firefox",
testMatch: ["tests/**/*.spec.ts", SHARED_SPECS],
testIgnore: EXTENSION_SPECS,
use: { ...devices["Desktop Firefox"], baseURL: BASE_URL },
},
Expand All @@ -276,7 +200,21 @@ export default defineConfig({
// loud startup error instead; set UI_LOOP_PORT / UI_LOOP_EXTENSION_PORT to run
// alongside a dev server you want to keep.
webServer: LIVE
? LIVE_WEB_SERVERS
? [
{
command: LIVE_COMMAND,
url: LIVE_BASE_URL,
reuseExistingServer: false,
timeout: 120_000,
// Whatever starts the live server is the most useful output a failed
// live run has — something that cannot reach the backend says so there,
// and Playwright discards a web server's stdout unless asked to pass it
// through.
stdout: "pipe",
stderr: "pipe",
env: LIVE_APP,
},
]
: [
{
command: `yarn dev --port ${PORT}`,
Expand Down
Loading
Loading