-
Notifications
You must be signed in to change notification settings - Fork 104
test: cover cross-process credential writes #280
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
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,98 @@ | ||
| import { spawn } from 'node:child_process'; | ||
| import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; | ||
| import { tmpdir } from 'node:os'; | ||
| import { dirname, join, resolve } from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
| import { beforeAll, describe, expect, it } from 'vitest'; | ||
| import { readCredentialsFile } from '../src/lib/credentials.js'; | ||
| import { execNpm } from './helpers/execNpm.js'; | ||
|
|
||
| const __dirname = dirname(fileURLToPath(import.meta.url)); | ||
| const REPO_ROOT = resolve(__dirname, '..'); | ||
| const CHILD_PATH = join(REPO_ROOT, 'test', 'helpers', 'credentials-write-child.mjs'); | ||
|
|
||
| interface ChildResult { | ||
| code: number | null; | ||
| signal: NodeJS.Signals | null; | ||
| stderr: string; | ||
| stdout: string; | ||
| } | ||
|
|
||
| function runCredentialWriter(env: Record<string, string>): Promise<ChildResult> { | ||
| return new Promise((resolveChild, reject) => { | ||
| const child = spawn(process.execPath, [CHILD_PATH], { | ||
| cwd: REPO_ROOT, | ||
| env: { ...process.env, ...env }, | ||
| stdio: ['ignore', 'pipe', 'pipe'], | ||
| }); | ||
| const stdout: Buffer[] = []; | ||
| const stderr: Buffer[] = []; | ||
| const timer = setTimeout(() => { | ||
| child.kill(); | ||
| }, 10_000); | ||
|
|
||
| child.stdout.on('data', chunk => stdout.push(Buffer.from(chunk))); | ||
| child.stderr.on('data', chunk => stderr.push(Buffer.from(chunk))); | ||
| child.on('error', error => { | ||
| clearTimeout(timer); | ||
| reject(error); | ||
| }); | ||
| child.on('close', (code, signal) => { | ||
| clearTimeout(timer); | ||
| resolveChild({ | ||
| code, | ||
| signal, | ||
| stdout: Buffer.concat(stdout).toString('utf8'), | ||
| stderr: Buffer.concat(stderr).toString('utf8'), | ||
| }); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| describe('credentials cross-process writes', () => { | ||
| beforeAll(() => { | ||
| execNpm(['run', 'build'], { cwd: REPO_ROOT, stdio: 'pipe' }); | ||
| }); | ||
|
|
||
| it('preserves every profile written by concurrent child processes', async () => { | ||
| const tmpRoot = mkdtempSync(join(tmpdir(), 'testsprite-creds-race-')); | ||
| const credentialsPath = join(tmpRoot, 'credentials'); | ||
| const startPath = join(tmpRoot, 'start'); | ||
|
|
||
| try { | ||
| const profiles = Array.from({ length: 8 }, (_, index) => ({ | ||
| apiKey: `sk-child-${index}`, | ||
| profile: `child-${index}`, | ||
| })); | ||
| const children = profiles.map(({ apiKey, profile }) => | ||
| runCredentialWriter({ | ||
| CRED_API_KEY: apiKey, | ||
| CRED_PATH: credentialsPath, | ||
| CRED_PROFILE: profile, | ||
| CRED_START_PATH: startPath, | ||
| }), | ||
| ); | ||
|
|
||
| await new Promise(resolveDelay => setTimeout(resolveDelay, 50)); | ||
| writeFileSync(startPath, 'go'); | ||
|
|
||
| const results = await Promise.all(children); | ||
| expect(results).toEqual( | ||
| profiles.map(() => ({ | ||
| code: 0, | ||
| signal: null, | ||
| stderr: '', | ||
| stdout: '', | ||
| })), | ||
| ); | ||
|
|
||
| const credentials = readCredentialsFile({ path: credentialsPath }); | ||
| for (const { apiKey, profile } of profiles) { | ||
| expect(credentials[profile]).toEqual({ apiKey }); | ||
| } | ||
| expect(existsSync(`${credentialsPath}.lock`)).toBe(false); | ||
|
Comment on lines
+57
to
+93
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 | 🟠 Major | ⚡ Quick win Only the success path is covered; the helper's error/exit-code paths are untested. The child helper documents three distinct failure exit codes (1: missing env vars, 2: start-marker timeout, 3: writeProfile error), but this suite only ever exercises the As per path instructions, "Cover new behavior, including error and exit-code paths." Consider adding focused cases, e.g. omitting a required env var (expect code 1) and pointing 🧰 Tools🪛 ast-grep (0.44.1)[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec. (detect-child-process-typescript) 🤖 Prompt for AI AgentsSource: Path instructions |
||
| } finally { | ||
| rmSync(tmpRoot, { force: true, recursive: true }); | ||
| } | ||
| }, 20_000); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import { existsSync } from 'node:fs'; | ||
| import { writeProfile } from '../../dist/lib/credentials.js'; | ||
|
|
||
| const profile = process.env.CRED_PROFILE; | ||
| const credentialsPath = process.env.CRED_PATH; | ||
| const apiKey = process.env.CRED_API_KEY; | ||
| const startPath = process.env.CRED_START_PATH; | ||
|
|
||
| if (!profile || !credentialsPath || !apiKey || !startPath) { | ||
| console.error('CRED_PROFILE, CRED_PATH, CRED_API_KEY, and CRED_START_PATH are required'); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const deadline = Date.now() + 5_000; | ||
| while (!existsSync(startPath)) { | ||
| if (Date.now() >= deadline) { | ||
| console.error(`Timed out waiting for start marker: ${startPath}`); | ||
| process.exit(2); | ||
| } | ||
| Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 5); | ||
| } | ||
|
|
||
| try { | ||
| writeProfile(profile, { apiKey }, { path: credentialsPath }); | ||
| } catch (error) { | ||
| console.error(error instanceof Error ? error.stack : String(error)); | ||
| process.exit(3); | ||
| } |
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.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: TestSprite/testsprite-cli
Length of output: 5634
🌐 Web query:
vitest v2 default hookTimeout beforeAll timeout documentation💡 Result:
In Vitest, including v2, the default timeout for hooks like beforeAll is 10,000 milliseconds (10 seconds) when running in a Node.js environment [1][2][3]. If browser mode is enabled, the default timeout increases to 30,000 milliseconds (30 seconds) [1][3][4]. You can override this global setting using the hookTimeout configuration option [1][2]: // vitest.config.ts export default { test: { hookTimeout: 20000, // Sets hook timeout to 20 seconds }, } Additionally, you can specify a custom timeout for an individual beforeAll hook by passing the timeout as the second argument [2][5]: beforeAll(async => { // your setup logic }, 20000); // Individual timeout of 20 seconds Setting the timeout to 0 disables it completely [1][3][6].
Citations:
Set an explicit timeout for the
beforeAllbuild hook.This
beforeAllrunsnpm run buildviatscand a postbuild script, but the hook uses Vitest’s default 10s timeout. That can fail the test run with a generic hook-timeout error instead of a clear build failure.🔧 Suggested fix
📝 Committable suggestion
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents