-
-
Notifications
You must be signed in to change notification settings - Fork 127
Add a smoke test to generated fedify init apps
#990
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
base: main
Are you sure you want to change the base?
Changes from all commits
f9e6667
e133891
bc3bdf3
27a7063
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 |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| - Added a `test` task to projects scaffolded by `fedify init`. It starts | ||
| the app, waits for it to become ready, and checks that it resolves a local | ||
| actor, giving projects a standard smoke test to run right after scaffolding | ||
| and whenever the app changes afterwards. [[#898]] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| import { message } from "@optique/core"; | ||
| import assert from "node:assert/strict"; | ||
| import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; | ||
| import { tmpdir } from "node:os"; | ||
| import { join } from "node:path"; | ||
| import test from "node:test"; | ||
| import { message } from "@optique/core"; | ||
| import type { InitCommandData } from "../types.ts"; | ||
| import { | ||
| assertNoGeneratedFileConflicts, | ||
|
|
@@ -92,6 +92,18 @@ test("patchFiles merges JSONC files containing only comments", async () => { | |
| }); | ||
| }); | ||
|
|
||
| test("patchFiles writes the smoke-test script", async () => { | ||
| await withTempDir(async (dir) => { | ||
| await patchFiles(createInitData(dir, false)); | ||
|
|
||
| const testScript = await readFile( | ||
| join(dir, "scripts", "smokeTest.ts"), | ||
| "utf8", | ||
| ); | ||
| assert.match(testScript, /\["npm","run","dev"\]/); | ||
| }); | ||
| }); | ||
|
Comment on lines
+95
to
+105
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. 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Expect tests for Deno and Node.js or Bun smoke-test generation.
rg -n -C 4 \
'smokeTest|packageManager: "(deno|bun|npm|pnpm|yarn)"|test task|run.*dev' \
packages/init/src --glob '*.test.ts'Repository: fedify-dev/fedify Length of output: 15893 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Relevant patch.test.ts slices:"
sed -n '1,150p' packages/init/src/action/patch.test.ts
echo
echo "Tests mentioning smokeTest/runs/patchFiles/packageManager deno:"
rg -n -C 3 --glob '*.test.ts' \
'smokeTest|patchFiles|packageManager:\s*"?deno"?"|packageManager:\s*"?bun"?"|packageManager:\s*"?npm"?"|run.*dev|test task|tasks:\s*' packages/init/src/action packages/init/src/action/patch.test.tsRepository: fedify-dev/fedify Length of output: 25321 Add smoke-test command coverage for Deno.
🤖 Prompt for AI Agents |
||
|
|
||
| function createInitData( | ||
| dir: string, | ||
| allowNonEmpty: boolean, | ||
|
|
@@ -111,6 +123,7 @@ function createInitData( | |
| initializer: { | ||
| federationFile: "src/federation.ts", | ||
| loggingFile: "src/logging.ts", | ||
| testFile: "scripts/smokeTest.ts", | ||
| instruction: message`done`, | ||
| tasks: {}, | ||
| compilerOptions: {}, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import { concat, entries, join, map, pipe, when } from "@fxts/core"; | ||
| import { toMerged } from "es-toolkit"; | ||
| import { readTemplate } from "../lib.ts"; | ||
| import { getDevCommand, readTemplate } from "../lib.ts"; | ||
| import type { InitCommandData, PackageManager } from "../types.ts"; | ||
| import { replace } from "../utils.ts"; | ||
| import { needsDenoDotenv } from "./utils.ts"; | ||
|
|
@@ -57,6 +57,28 @@ export const loadLogging = async ( | |
| replace(/\/\* project name \*\//, JSON.stringify(projectName)), | ||
| ); | ||
|
|
||
| /** | ||
| * Loads the smoke-test script content for the initializer. | ||
| * | ||
| * Every framework shares the same *defaults/smokeTest.ts* template, so unlike | ||
| * {@link loadLogging} there is no per-framework template override. The | ||
| * template spawns the project's own dev server, so it needs the dev command | ||
| * for the chosen package manager baked in at generation time. | ||
| * | ||
| * @param param0 - {@link InitCommandData} containing `packageManager` | ||
| * @returns The complete smoke-test script content as a string | ||
| */ | ||
|
2chanhaeng marked this conversation as resolved.
|
||
| export const loadTest = async ( | ||
| { packageManager }: InitCommandData, | ||
| ) => | ||
| pipe( | ||
| await readTemplate("defaults/smokeTest.ts"), | ||
| replace( | ||
| /\/\* dev command \*\//, | ||
| JSON.stringify(getDevCommand(packageManager).split(" ")), | ||
|
Member
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. If converted as like this, the formatter will show that a formatting issue occurred, causing unnecessary confusion for the user. Add a space ( |
||
| ), | ||
| ); | ||
|
|
||
| /** | ||
| * Generates import statements for KV store and message queue dependencies. | ||
| * Merges imports from both KV and MQ configurations and creates proper | ||
|
|
||
|
Member
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. It seems like it would be better to either match the file name to kebab-case like the other file names, or name it |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,141 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { getDocumentLoader } from "@fedify/fedify"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { type Actor, isActor, lookupObject } from "@fedify/vocab"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { spawn } from "node:child_process"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| const DEV_COMMAND: string[] = /* dev command */; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const HANDLE = "john"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const STARTUP_TIMEOUT = 15_000; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| async function main(): Promise<void> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [command, ...args] = DEV_COMMAND; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const server = spawn(command, args, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| stdio: ["ignore", "pipe", "pipe"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| detached: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+11
to
+14
Member
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. I think this would be failed in Windows. Check |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| server.on("error", () => {}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| const exitOnSignal = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| stopServer(server); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| process.exit(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| process.once("SIGINT", exitOnSignal); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| process.once("SIGTERM", exitOnSignal); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| let output = ""; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const collectOutput = (chunk: Buffer) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| output += chunk.toString("utf8"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| server.stdout?.on("data", collectOutput); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| server.stderr?.on("data", collectOutput); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const port = await determinePort(server); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const target = `http://localhost:${port}/users/${HANDLE}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await waitForServer(target); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log(`Server is up at http://localhost:${port}.`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const actor = await checkActor(target); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log(actor); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log(`Smoke test passed: ${target} resolved to an actor.`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.error("Smoke test failed:", error instanceof Error ? error.message : error); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (output.trim() !== "") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.error(`\nDev server output:\n${output}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| process.exitCode = 1; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } finally { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| stopServer(server); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| function determinePort(server: ReturnType<typeof spawn>): Promise<number> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const portPatterns = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| /listening on.*:(\d+)/i, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| /server.*:(\d+)/i, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| /port\s*:?\s*(\d+)/i, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| /https?:\/\/localhost:(\d+)/i, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| /https?:\/\/0\.0\.0\.0:(\d+)/i, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| /https?:\/\/127\.0\.0\.1:(\d+)/i, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| /https?:\/\/[^:]+:(\d+)/i, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new Promise((resolve, reject) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const timeout = setTimeout(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| reject( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| new Error( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| `Timeout: Could not determine port from server output within ${STARTUP_TIMEOUT}ms.`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, STARTUP_TIMEOUT); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| const onData = (chunk: Buffer) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const text = chunk.toString("utf8"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const pattern of portPatterns) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const match = text.match(pattern); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (match && match[1]) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const port = Number.parseInt(match[1], 10); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| clearTimeout(timeout); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| resolve(port); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+70
to
+79
Member
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. The code searches a port within individual chunks. This might cause the banner to be split, potentially preventing the port from being found even if the server is operating normally. Please refer to |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+69
to
+80
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. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Buffer server output before matching the port.
Proposed fix+ let portOutput = "";
const onData = (chunk: Buffer) => {
- const text = chunk.toString("utf8");
+ portOutput += chunk.toString("utf8");
for (const pattern of portPatterns) {
- const match = text.match(pattern);
+ const match = portOutput.match(pattern);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| server.stdout?.on("data", onData); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| server.stderr?.on("data", onData); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| server.once("exit", (code) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| clearTimeout(timeout); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| reject(new Error(`The dev server exited early with code ${String(code)}.`)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| async function waitForServer(url: string): Promise<void> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const startTime = Date.now(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let lastStatus: number | undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| while (Date.now() - startTime < STARTUP_TIMEOUT) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const response = await fetch(url, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers: { Accept: "application/activity+json" }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| signal: AbortSignal.timeout(1000), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await response.body?.cancel(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (response.ok) return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| lastStatus = response.status; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Server not ready yet, continue waiting | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await new Promise((resolve) => setTimeout(resolve, 500)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| `The server did not become ready within ${STARTUP_TIMEOUT}ms.` + | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| (lastStatus == null ? "" : ` Last response status: ${lastStatus}.`), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| async function checkActor(url: string): Promise<Actor> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const object = await lookupObject(url, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| documentLoader: getDocumentLoader({ allowPrivateAddress: true }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+115
to
+118
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. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the local Fedify implementation for timeout or abort support.
rg -n -C 5 --glob '*.ts' \
'lookupObject|function getDocumentLoader|const getDocumentLoader' .Repository: fedify-dev/fedify Length of output: 50375 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== template excerpt =="
sed -n '80,140p' packages/init/src/templates/defaults/smokeTest.ts.tpl
echo
echo "== lookup API excerpt =="
sed -n '90,285p' packages/vocab/src/lookup.ts
echo
echo "== all lookupObject calls in smoke template =="
rg -n "lookupObject|signal|AbortController|setTimeout|checkActor|test\\(" packages/init/src/templates/defaults/smokeTest.ts.tplRepository: fedify-dev/fedify Length of output: 8802 Apply the startup timeout to actor resolution.
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (object == null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`Could not resolve an actor at ${url}.`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!isActor(object)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`Expected an actor at ${url}, but got a non-actor object.`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return object; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| function stopServer(server: ReturnType<typeof spawn>): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (server.pid != null) process.kill(-server.pid, "SIGKILL"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
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. Process tree termination handling for Windows is required. Because Windows does not support terminating a process group using a negative PID. |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Process group already exited. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| server.kill("SIGKILL"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Process already exited. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| await main(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
Add this pull request number and also your name here!