Repository navigation
fix(scripts): scenarios reach the screen and say why they could not run #36054
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,6 +53,11 @@ export interface SideResult { | |
| readonly status: "passed" | "failed" | "error"; | ||
| readonly title: string; | ||
| readonly message?: string; | ||
| /** | ||
| * For `error`, what stopped: `app` when the app did not start or no test ran, `scenario` when | ||
| * the app ran and the scenario stopped before its assertion, such as on a control it did not find. | ||
| */ | ||
| readonly stopped?: "app" | "scenario"; | ||
| readonly steps: ReadonlyArray<{ readonly step: string; readonly status: string }>; | ||
| /** Capture paths under the run's results directory, in the order the scenario took them. */ | ||
| readonly captures: ReadonlyArray<string>; | ||
|
|
@@ -75,7 +80,16 @@ export function readSide( | |
| executed.length > 1 | ||
| ? `the scenario file ran ${executed.length} tests; a proof needs exactly one` | ||
| : "e2e wrote no report: the app did not start, or no test was selected"; | ||
| return { sha, status: "error", title: "", message, steps: [], captures: [], videos: [] }; | ||
| return { | ||
| sha, | ||
| status: "error", | ||
| stopped: "app", | ||
| title: "", | ||
| message, | ||
| steps: [], | ||
| captures: [], | ||
| videos: [], | ||
| }; | ||
| } | ||
| const files = (kind: string) => | ||
| (attempt.artifacts ?? []).flatMap((artifact) => | ||
|
|
@@ -84,15 +98,18 @@ export function readSide( | |
| : [], | ||
| ); | ||
| const error = attempt.error; | ||
| // e2e exits 0 for a skipped test too, and a test that did not run proves nothing. | ||
| const status = | ||
| attempt.status === "passed" | ||
| ? "passed" | ||
| : error?.code === "ASSERTION_FAILED" | ||
| ? "failed" | ||
| : "error"; | ||
| return { | ||
| sha, | ||
| // e2e exits 0 for a skipped test too, and a test that did not run proves nothing. | ||
| status: | ||
| attempt.status === "passed" | ||
| ? "passed" | ||
| : error?.code === "ASSERTION_FAILED" | ||
| ? "failed" | ||
| : "error", | ||
| status, | ||
| // A test that ended with an error of its own got as far as the app. | ||
| ...(status === "error" ? { stopped: error === undefined ? "app" : "scenario" } : {}), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Classify startup error codes as app stops. An attempt error from app startup or browser launch currently becomes Suggested fix
|
||
| title: result.titlePath.join(" > "), | ||
| ...(error === undefined | ||
| ? attempt.status === "passed" | ||
|
|
@@ -123,7 +140,7 @@ export function buildProof(base: SideResult, head: SideResult, link: (path: stri | |
| const broken = base.status === "error" ? base : head.status === "error" ? head : undefined; | ||
| const verdict = | ||
| broken !== undefined | ||
| ? `Not proven: the scenario could not run on ${broken === base ? `base ${baseSha}` : `head ${headSha}`} (${broken.message ?? "unknown error"}).` | ||
| ? `Not proven: the scenario could not run on ${broken === base ? `base ${baseSha}` : `head ${headSha}`} (${broken.message ?? "unknown error"}).${broken.stopped === "scenario" ? " The app ran and the scenario stopped before its assertion: fix the scenario and run it again." : ""}` | ||
| : base.status === "passed" | ||
| ? head.status === "passed" | ||
| ? `Not proven: the scenario passes on base ${baseSha} too, so it does not reproduce the bug.` | ||
|
|
@@ -160,6 +177,8 @@ export function buildProof(base: SideResult, head: SideResult, link: (path: stri | |
| scenario: head.title || base.title, | ||
| failsOnBase: base.status === "failed", | ||
| passesOnHead: head.status === "passed", | ||
| /** Why there is no verdict on the behavior: the app did not run, or the scenario stopped. */ | ||
| couldNotRun: broken?.stopped ?? null, | ||
| baseSha: base.sha, | ||
| headSha: head.sha, | ||
| verdict, | ||
|
|
@@ -192,10 +211,18 @@ const withCheckout = Effect.fn("prove.withCheckout")(function* <A, E, R>( | |
| if (clean && (yield* git(["rev-parse", "HEAD"])) === sha) return yield* use(repo); | ||
| // Beside this checkout, not in the temp directory, so tooling resolves as it does here. | ||
| const worktree = `${repo}-scenario-${short(sha)}`; | ||
| // A run that was killed left its worktree behind, and git refuses to add over it. | ||
| const discard = git(["worktree", "remove", "--force", worktree]).pipe( | ||
| Effect.ignore, | ||
| Effect.andThen(git(["worktree", "prune"])), | ||
| Effect.ignore, | ||
| ); | ||
| return yield* Effect.acquireUseRelease( | ||
| git(["worktree", "add", "--quiet", "--detach", "--force", worktree, sha]), | ||
| discard.pipe( | ||
| Effect.andThen(git(["worktree", "add", "--quiet", "--detach", "--force", worktree, sha])), | ||
| ), | ||
| () => run("vp", ["i"], { cwd: worktree, echo: true }).pipe(Effect.andThen(use(worktree))), | ||
| () => git(["worktree", "remove", "--force", worktree]).pipe(Effect.ignore), | ||
| () => discard, | ||
| ); | ||
| }); | ||
|
|
||
|
|
@@ -204,6 +231,7 @@ const runSide = Effect.fn("prove.runSide")(function* ( | |
| side: "base" | "head", | ||
| rev: string, | ||
| scenarioPath: string, | ||
| target: string, | ||
| ) { | ||
| const fs = yield* FileSystem.FileSystem; | ||
| const path = yield* Path.Path; | ||
|
|
@@ -216,7 +244,7 @@ const runSide = Effect.fn("prove.runSide")(function* ( | |
| // The scenario and e2e come from this checkout; only the app comes from `appDir`. | ||
| run( | ||
| path.join(scripts, "node_modules/.bin/e2e"), | ||
| ["run", scenarioPath, "--output", path.relative(scripts, outDir)], | ||
| ["run", scenarioPath, "--target", target, "--output", path.relative(scripts, outDir)], | ||
| { | ||
| cwd: scripts, | ||
| env: { ...environment, T3_SCENARIO_APP_DIR: appDir, E2E_TELEMETRY_DISABLED: "1" }, | ||
|
|
@@ -230,7 +258,22 @@ const runSide = Effect.fn("prove.runSide")(function* ( | |
| Effect.flatMap(decodeReport), | ||
| Effect.orElseSucceed(() => undefined), | ||
| ); | ||
| return readSide(sha, path.join(outDir, "results"), report); | ||
| const result = readSide(sha, path.join(outDir, "results"), report); | ||
| // Written as each side ends, so a proof whose second side is cut short still has its first. | ||
| yield* fs | ||
| .writeFileString( | ||
| path.join(scripts, ".e2e/prove", `${side}.json`), | ||
| `${yield* encodeProof(result)}\n`, | ||
| ) | ||
| .pipe(Effect.ignore); | ||
| return result; | ||
| }); | ||
|
|
||
| /** The one target a scenario runs on: an iOS scenario imports `t3-mobile.ts`, a web one does not. */ | ||
| const targetOf = Effect.fn("prove.targetOf")(function* (scenarioPath: string) { | ||
| const fs = yield* FileSystem.FileSystem; | ||
| const source = yield* fs.readFileString(scenarioPath).pipe(Effect.orElseSucceed(() => "")); | ||
| return source.includes("t3-mobile") ? "ios" : "web"; | ||
| }); | ||
|
|
||
| /** Uploads the captures and maps each to its URL; a failed upload leaves the map empty. */ | ||
|
|
@@ -262,6 +305,7 @@ const prove = Effect.fn("prove")(function* (input: { | |
| readonly base: string; | ||
| readonly head: string; | ||
| readonly uploadCommand: Option.Option<string>; | ||
| readonly target: Option.Option<string>; | ||
| }) { | ||
| const fs = yield* FileSystem.FileSystem; | ||
| const path = yield* Path.Path; | ||
|
|
@@ -271,8 +315,10 @@ const prove = Effect.fn("prove")(function* (input: { | |
| // A proof left by an earlier run must not be read as this one's. | ||
| yield* fs.remove(proveDir, { recursive: true, force: true }); | ||
| yield* fs.makeDirectory(proveDir, { recursive: true }); | ||
| const base = yield* runSide("base", input.base, scenarioPath); | ||
| const head = yield* runSide("head", input.head, scenarioPath); | ||
| // Only that target's app starts: a web scenario needs no Metro and no device. | ||
| const target = Option.getOrElse(input.target, () => "") || (yield* targetOf(scenarioPath)); | ||
| const base = yield* runSide("base", input.base, scenarioPath, target); | ||
| const head = yield* runSide("head", input.head, scenarioPath, target); | ||
| const links = Option.isSome(input.uploadCommand) | ||
| ? yield* upload(input.uploadCommand.value, [...base.captures, ...head.captures]) | ||
| : new Map<string, string>(); | ||
|
|
@@ -285,8 +331,13 @@ const prove = Effect.fn("prove")(function* (input: { | |
| yield* fs.writeFileString(path.join(proveDir, "proof.md"), proof.markdown); | ||
| yield* fs.writeFileString(path.join(proveDir, "proof.json"), `${json}\n`); | ||
| yield* Console.log(json); | ||
| const couldNotRun = base.status === "error" || head.status === "error"; | ||
| return proof.failsOnBase && proof.passesOnHead ? 0 : couldNotRun ? 2 : 1; | ||
| return proof.failsOnBase && proof.passesOnHead | ||
| ? 0 | ||
| : proof.couldNotRun === "scenario" | ||
| ? 3 | ||
| : proof.couldNotRun === "app" | ||
| ? 2 | ||
| : 1; | ||
| }); | ||
|
|
||
| const command = Command.make( | ||
|
|
@@ -304,6 +355,12 @@ const command = Command.make( | |
| Flag.withDescription("Command that takes capture paths and prints one URL per line."), | ||
| Flag.optional, | ||
| ), | ||
| target: Flag.String("target").pipe( | ||
| Flag.withDescription( | ||
| "Target to run on: web or ios. Default: ios when the scenario imports t3-mobile.ts, else web.", | ||
| ), | ||
| Flag.optional, | ||
| ), | ||
| }, | ||
| (input) => | ||
| prove(input).pipe( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,11 +2,13 @@ | |
| // app. `t3-mobile.ts` is the same for the iOS app. | ||
| import * as NodeServices from "@effect/platform-node/NodeServices"; | ||
| import { test as base } from "@e2e-dev/web"; | ||
| import { expect } from "e2e"; | ||
| import * as HostProcess from "@t3tools/shared/HostProcess"; | ||
| import * as Effect from "effect/Effect"; | ||
| import * as FileSystem from "effect/FileSystem"; | ||
| import * as Path from "effect/Path"; | ||
| import * as Schema from "effect/Schema"; | ||
| import { FetchHttpClient, HttpClient, HttpClientResponse } from "effect/http"; | ||
|
|
||
| import { APP_DIR, T3_HOME } from "../e2e.config.ts"; | ||
| import { run } from "./shell.ts"; | ||
|
|
@@ -23,12 +25,19 @@ export interface T3State { | |
| readonly title: string; | ||
| readonly messages: ReadonlyArray<string>; | ||
| readonly model?: string; | ||
| }) => Promise<void>; | ||
| }) => Promise<T3Thread>; | ||
| } | ||
|
|
||
| export interface T3Thread { | ||
| readonly threadId: string; | ||
| } | ||
|
|
||
| export interface T3 extends T3State { | ||
| /** Opens the app, signed in, at `path`. */ | ||
| readonly open: (path?: string) => Promise<void>; | ||
| /** | ||
| * Opens the app, signed in, at a path or on a thread `t3.thread` returned. Opening the thread | ||
| * by its link is the reliable way to reach it: a sidebar row covers its own title text. | ||
| */ | ||
| readonly open: (target?: string | T3Thread) => Promise<void>; | ||
| } | ||
|
|
||
| /** e2e's fixtures are promise functions; this is the one place an Effect becomes a promise. */ | ||
|
|
@@ -99,6 +108,7 @@ export function makeState(home: string, env: NodeJS.ProcessEnv = {}) { | |
| const started = yield* turn(["start", input.project, first, "--title", input.title, ...replay]); | ||
| const { threadId } = yield* decodeLaunched(started.stdout); | ||
| for (const message of rest) yield* turn(["send", threadId, message]); | ||
| return { threadId } satisfies T3Thread; | ||
| }); | ||
|
|
||
| return { | ||
|
|
@@ -122,14 +132,31 @@ const readPairingPath = Effect.gen(function* () { | |
| return yield* fs.readFileString(path.join(T3_HOME, "pairing-path")); | ||
| }); | ||
|
|
||
| /** The id every thread link starts with, asked of the server the app talks to. */ | ||
| const environmentId = (baseUrl: string) => | ||
| HttpClient.get(`${baseUrl}/.well-known/t3/environment`).pipe( | ||
| Effect.flatMap(HttpClientResponse.filterStatusOk), | ||
| Effect.flatMap( | ||
| HttpClientResponse.schemaBodyJson(Schema.Struct({ environmentId: Schema.String })), | ||
| ), | ||
| Effect.map((environment) => environment.environmentId), | ||
| Effect.provide(FetchHttpClient.layer), | ||
| ); | ||
|
|
||
| export const test = base.extend<{ t3: T3 }>({ | ||
| t3: async ({ app }, use) => { | ||
| t3: async ({ app, browser }, use) => { | ||
| const { state, cleanup } = makeState(T3_HOME); | ||
| await use({ | ||
| ...state, | ||
| open: async (path = "/") => { | ||
| open: async (target = "/") => { | ||
| await app.open(await runPromise(readPairingPath)); | ||
| if (path !== "/") await app.open(path); | ||
| // The pairing page signs in and then leaves itself. Going anywhere before it has done | ||
| // so cancels the sign-in, and the next page is the pairing page again. | ||
| await expect(browser).not.toHaveURL(/\/pair(?:[?#]|$)/, { timeout: 60_000 }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not classify pairing readiness as a bug assertion. If pairing remains on Use a non-assertion navigation wait. Report a setup error when that wait fails, so the proof classifies the stop as
|
||
| if (typeof target !== "string") { | ||
| const environment = await runPromise(environmentId(app.baseUrl ?? "")); | ||
| await app.open(`/${environment}/${target.threadId}`); | ||
| } else if (target !== "/") await app.open(target); | ||
| }, | ||
| }); | ||
| await runPromise(cleanup); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These inputs are already covered by the verdict table above. Could you fold the
couldNotRunassertions into those existing rows instead of adding a separate test?Posted via Macroscope — Test Signal