Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 40 additions & 0 deletions .github/workflows/test-database.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
name: Database tests
on:
pull_request:
paths:
- "packages/database/**"
env:
SUPABASE_USE_DB: local
SUPABASE_PROJECT_ID: test
GITHUB_TEST: test

jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v6
- name: Install pnpm
uses: pnpm/action-setup@v4
with:
version: 10.15.1
run_install: false
- name: Setup node
uses: actions/setup-node@v4
with:
node-version: "20"
cache: "pnpm"
- name: Install Dependencies
run: pnpm install --frozen-lockfile
# - name: Get supabase version
# working-directory: ./packages/database
# run: echo "SUPABASE_VERSION=$(./node_modules/.bin/supabase --version)" >> $GITHUB_ENV
# - name: Cache Docker images.
# uses: AndreKurait/docker-cache@0.6.0
# with:
# key: docker-${{ runner.os }}-${{ env.SUPABASE_VERSION }}
- name: Setup database
working-directory: ./packages/database
run: pnpm run setup
- name: Serve and run tests
working-directory: ./packages/database
run: pnpm run test:withserve
1 change: 1 addition & 0 deletions packages/database/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"lint:fix": "eslint --fix . && tsx scripts/lintSchemas.ts -f && tsx scripts/lintFunctions.ts",
"migrate": "tsx scripts/migrate.ts",
"test": "pnpm run build && cucumber-js",
"test:withserve": "pnpm run build && tsx scripts/serve_and_test.ts",
"genenv": "tsx scripts/createEnv.mts",
"gentypes": "tsx scripts/genTypes.ts",
"dbdiff": "supabase stop && supabase db diff --use-pg-schema",
Expand Down
41 changes: 25 additions & 16 deletions packages/database/scripts/createEnv.mts
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,17 @@ const getVercelToken = () => {
};

const makeFnEnv = (envTxt: string): string => {
return envTxt.split('\n').filter(l=>l.match(/^SUPABASE_\w+_KEY/)).map((l)=> l.replace('SUPABASE_', 'SB_')).join('\n');
}
return envTxt
.split("\n")
.filter((l) => l.match(/^SUPABASE_\w+_KEY/))
.map((l) => l.replace("SUPABASE_", "SB_"))
.join("\n");
};

const makeLocalEnv = () => {
execSync("supabase start", {
cwd: projectRoot, stdio: "inherit"
cwd: projectRoot,
stdio: "inherit",
});
const stdout = execSync("supabase status -o env", {
encoding: "utf8",
Expand All @@ -54,8 +59,8 @@ const makeLocalEnv = () => {
);
writeFileSync(
join(projectRoot, "supabase/functions/.env"),
makeFnEnv(prefixed)
)
makeFnEnv(prefixed),
);
};

const makeBranchEnv = async (vercel: Vercel, vercelToken: string) => {
Expand Down Expand Up @@ -94,11 +99,11 @@ const makeBranchEnv = async (vercel: Vercel, vercelToken: string) => {
throw err;
}
appendFileSync(".env.branch", `NEXT_API_ROOT="https://${url}/api"\n`);
const fromVercel = readFileSync('.env.branch').toString();
const fromVercel = readFileSync(".env.branch").toString();
writeFileSync(
join(projectRoot, "supabase/functions/.env"),
makeFnEnv(fromVercel)
)
makeFnEnv(fromVercel),
);
};

const makeProductionEnv = async (vercel: Vercel, vercelToken: string) => {
Expand All @@ -117,33 +122,37 @@ const makeProductionEnv = async (vercel: Vercel, vercelToken: string) => {
`vercel -t ${vercelToken} env pull --environment production .env.production`,
);
appendFileSync(".env.production", `NEXT_API_ROOT="https://${url}/api"\n`);
const fromVercel = readFileSync('.env.production').toString();
const fromVercel = readFileSync(".env.production").toString();
writeFileSync(
join(projectRoot, "supabase/functions/.env"),
makeFnEnv(fromVercel)
)
makeFnEnv(fromVercel),
);
};

const main = async (variant: Variant) => {
if (process.env.ROAM_BUILD_SCRIPT) {
// special case: production build
try {
const response = execSync('curl https://discoursegraphs.com/api/supabase/env');
const response = execSync(
"curl https://discoursegraphs.com/api/supabase/env",
);
const asJson = JSON.parse(response.toString()) as Record<string, string>;
writeFileSync(
join(projectRoot, ".env"),
Object.entries(asJson).map(([k,v])=>`${k}=${v}`).join('\n')
Object.entries(asJson)
.map(([k, v]) => `${k}=${v}`)
.join("\n"),
);
return;
} catch (e) {
if (process.env.SUPABASE_URL && process.env.SUPABASE_PUBLISHABLE_KEY)
return;
throw new Error("Could not get environment from site");
}
}
else if (
} else if (
process.env.HOME === "/vercel" ||
process.env.GITHUB_ACTIONS !== undefined
(process.env.GITHUB_ACTIONS !== undefined &&
process.env.GITHUB_TEST !== "test")
)
// Do not execute in deployment or github action.
return;
Expand Down
5 changes: 4 additions & 1 deletion packages/database/scripts/migrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import { getVariant } from "@repo/database/dbDotEnv";
const __dirname = dirname(__filename);
const projectRoot = join(__dirname, "..");

if (process.env.HOME === "/vercel" || process.env.GITHUB_ACTIONS === "true") {
if (
process.env.HOME === "/vercel" ||
(process.env.GITHUB_ACTIONS === "true" && process.env.GITHUB_TEST !== "test")
) {
console.log("Skipping in production environment");
process.exit(0);
}
Expand Down
80 changes: 80 additions & 0 deletions packages/database/scripts/serve_and_test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { spawn, execSync } from "node:child_process";
import { join, dirname } from "path";

const scriptDir = dirname(__filename);
const projectRoot = join(scriptDir, "..");

if (
process.env.GITHUB_ACTIONS === "true" &&
process.env.GITHUB_TEST !== "test"
) {
console.error("Please set the GITHUB_TEST variable to 'test'");
process.exit(2);
}
if (process.env.SUPABASE_PROJECT_ID !== "test") {
console.error("Please set the SUPABASE_PROJECT_ID variable to 'test'");
process.exit(2);
}

const serve = spawn("supabase", ["functions", "serve"], {
cwd: projectRoot,
detached: true,
});
Comment on lines +19 to +22
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Unconsumed stderr pipe can deadlock the supabase serve child process

The serve child process is spawned with the default stdio: 'pipe' for all streams (serve_and_test.ts:19-22). While stdout has a 'data' listener that drains it, stderr has no listener at all. If supabase functions serve writes more than the OS pipe buffer size (~64 KB on Linux) to stderr before writing "Serving functions" to stdout, the child process will block on its stderr write, never produce the expected stdout output, and the script will hit the 30-second timeout. This is a well-known Node.js child-process pitfall. The fix is to either add serve.stderr.resume() to drain stderr, pipe stderr to 'inherit', or set it to 'ignore'.

Suggested change
const serve = spawn("supabase", ["functions", "serve"], {
cwd: projectRoot,
detached: true,
});
const serve = spawn("supabase", ["functions", "serve"], {
cwd: projectRoot,
detached: true,
stdio: ["ignore", "pipe", "inherit"],
});
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


let resolveCallback: ((value: unknown) => void) | undefined = undefined;
let rejectCallback: ((reason: unknown) => void) | undefined = undefined;
let serveSuccess = false;
let timeoutClear: NodeJS.Timeout | undefined = undefined;

const servingReady = new Promise((rsc, rjc) => {
resolveCallback = rsc;
rejectCallback = rjc;

// Add timeout
timeoutClear = setTimeout(() => {
rjc(new Error("Timeout waiting for functions to serve"));
}, 30000); // 30 second timeout
});

serve.stdout.on("data", (data: Buffer) => {
const output = data.toString();
console.log(`stdout: ${output}`);
if (output.includes("Serving functions ")) {
console.log("Found serving functions");
serveSuccess = true;
clearTimeout(timeoutClear);
if (resolveCallback === undefined) throw new Error("did not get callback");
resolveCallback(null);
}
});
serve.on("close", () => {
if (!serveSuccess && rejectCallback)
rejectCallback(new Error("serve closed without being ready"));
});
serve.on("error", (err) => {
if (rejectCallback) rejectCallback(err);
});

const doTest = async () => {
await servingReady;
try {
execSync("cucumber-js", {
cwd: projectRoot,
stdio: "inherit",
});
// will throw on failure
} finally {
if (serve.pid) process.kill(-serve.pid);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The process.kill(-serve.pid) call can throw an error if the process has already exited, which will mask any error from the test execution since this is in a finally block. This should be wrapped in a try-catch:

finally {
  if (serve.pid) {
    try {
      process.kill(-serve.pid);
    } catch (e) {
      // Process already exited, ignore
    }
  }
}
Suggested change
if (serve.pid) process.kill(-serve.pid);
if (serve.pid) {
try {
process.kill(-serve.pid);
} catch (e) {
// Process already exited, ignore
}
}

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 process.kill(-serve.pid) in finally block throws if process group already exited, masking test results

If the supabase functions serve process exits before process.kill(-serve.pid) is called at line 67, the call throws an ESRCH error. Because this is in a finally block, the thrown error replaces whatever result doTest() would have returned. Critically, if all tests pass but the serve process happens to have exited already, the ESRCH error propagates to the .catch() handler at line 76-79, which calls process.exit(1) — causing CI to report failure despite passing tests.

Suggested change
if (serve.pid) process.kill(-serve.pid);
try { if (serve.pid) process.kill(-serve.pid); } catch { /* already exited */ }
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}
};
Comment on lines +58 to +69
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Detached serve process never killed when servingReady rejects (timeout or early close)

In serve_and_test.ts, await servingReady at line 59 is outside the inner try/finally block (lines 60-68). If the servingReady promise rejects — most notably when the 30-second timeout fires at line 34-36, but the serve process is still running — the finally block containing process.kill(-serve.pid) is never reached. The rejection propagates directly to the .catch() handler at line 76 which calls process.exit(1) without killing the detached child. Because the child was spawned with detached: true (line 21), it survives the parent's exit and continues running as an orphaned process. In local development this leaves a lingering supabase functions serve process that holds the port.

Suggested change
const doTest = async () => {
await servingReady;
try {
execSync("cucumber-js", {
cwd: projectRoot,
stdio: "inherit",
});
// will throw on failure
} finally {
if (serve.pid) process.kill(-serve.pid);
}
};
const doTest = async () => {
try {
await servingReady;
execSync("cucumber-js", {
cwd: projectRoot,
stdio: "inherit",
});
// will throw on failure
} finally {
if (serve.pid) process.kill(-serve.pid);
}
};
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


doTest()
.then(() => {
console.log("success");
clearTimeout(timeoutClear);
})
.catch((err) => {
console.error(err);
clearTimeout(timeoutClear);
process.exit(1);
});
16 changes: 11 additions & 5 deletions packages/database/src/dbDotEnv.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { readFileSync, existsSync } from "node:fs";
import { join, dirname, basename } from "node:path";
import process from "node:process";
import console from "node:console";
import { fileURLToPath } from "node:url";
import dotenv from "dotenv";

Expand All @@ -14,7 +16,6 @@ const findRoot = () => {
}
return dir;
};

export const getVariant = () => {
const useDbArgPos = (process.argv || []).indexOf("--use-db");
let variant =
Expand All @@ -23,9 +24,8 @@ export const getVariant = () => {
: process.env["SUPABASE_USE_DB"];
if (variant === undefined) {
dotenv.config();
const dbGlobalEnv = join(findRoot(),'.env');
if (existsSync(dbGlobalEnv))
dotenv.config({path: dbGlobalEnv});
const dbGlobalEnv = join(findRoot(), ".env");
if (existsSync(dbGlobalEnv)) dotenv.config({ path: dbGlobalEnv });
variant = process.env["SUPABASE_USE_DB"];
}
const processHasVars =
Expand All @@ -39,7 +39,11 @@ export const getVariant = () => {
throw new Error("Invalid variant: " + variant);
}

if (process.env.HOME === "/vercel" || process.env.GITHUB_ACTIONS === "true") {
if (
process.env.HOME === "/vercel" ||
(process.env.GITHUB_ACTIONS === "true" &&
process.env.GITHUB_TEST !== "test")
) {
// deployment should have variables
if (!processHasVars) {
console.error("Missing SUPABASE variables in deployment");
Expand Down Expand Up @@ -76,9 +80,11 @@ export const envContents = () => {
if (!path) {
// Fallback to process.env when running in production environments
const raw = {
/* eslint-disable @typescript-eslint/naming-convention */
SUPABASE_URL: process.env.SUPABASE_URL,
SUPABASE_PUBLISHABLE_KEY: process.env.SUPABASE_PUBLISHABLE_KEY,
NEXT_API_ROOT: process.env.NEXT_API_ROOT,
/* eslint-enable @typescript-eslint/naming-convention */
};
return Object.fromEntries(Object.entries(raw).filter(([, v]) => !!v));
}
Expand Down
1 change: 1 addition & 0 deletions turbo.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"GEMINI_API_KEY",
"GH_CLIENT_SECRET_PROD",
"GITHUB_ACTIONS",
"GITHUB_TEST",
"HOME",
"OPENAI_API_KEY",
"POSTGRES_PASSWORD",
Expand Down
Loading