-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
test(node): Enable additionalDependencies in integration runner #17361
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 |
---|---|---|
@@ -0,0 +1,75 @@ | ||
import * as Sentry from '@sentry/node'; | ||
import { generateText } from 'ai'; | ||
import { MockLanguageModelV2 } from 'ai/test'; | ||
import { z } from 'zod'; | ||
|
||
async function run() { | ||
await Sentry.startSpan({ op: 'function', name: 'main' }, async () => { | ||
await generateText({ | ||
model: new MockLanguageModelV2({ | ||
doGenerate: async () => ({ | ||
finishReason: 'stop', | ||
usage: { inputTokens: 10, outputTokens: 20, totalTokens: 30 }, | ||
content: [{ type: 'text', text: 'First span here!' }], | ||
}), | ||
}), | ||
prompt: 'Where is the first span?', | ||
}); | ||
|
||
// This span should have input and output prompts attached because telemetry is explicitly enabled. | ||
await generateText({ | ||
experimental_telemetry: { isEnabled: true }, | ||
model: new MockLanguageModelV2({ | ||
doGenerate: async () => ({ | ||
finishReason: 'stop', | ||
usage: { inputTokens: 10, outputTokens: 20, totalTokens: 30 }, | ||
content: [{ type: 'text', text: 'Second span here!' }], | ||
}), | ||
}), | ||
prompt: 'Where is the second span?', | ||
}); | ||
|
||
// This span should include tool calls and tool results | ||
await generateText({ | ||
model: new MockLanguageModelV2({ | ||
doGenerate: async () => ({ | ||
finishReason: 'tool-calls', | ||
usage: { inputTokens: 15, outputTokens: 25, totalTokens: 40 }, | ||
content: [{ type: 'text', text: 'Tool call completed!' }], | ||
toolCalls: [ | ||
{ | ||
toolCallType: 'function', | ||
toolCallId: 'call-1', | ||
toolName: 'getWeather', | ||
args: '{ "location": "San Francisco" }', | ||
}, | ||
], | ||
}), | ||
}), | ||
tools: { | ||
getWeather: { | ||
parameters: z.object({ location: z.string() }), | ||
execute: async args => { | ||
return `Weather in ${args.location}: Sunny, 72°F`; | ||
}, | ||
}, | ||
}, | ||
prompt: 'What is the weather in San Francisco?', | ||
}); | ||
|
||
// This span should not be captured because we've disabled telemetry | ||
await generateText({ | ||
experimental_telemetry: { isEnabled: false }, | ||
model: new MockLanguageModelV2({ | ||
doGenerate: async () => ({ | ||
finishReason: 'stop', | ||
usage: { inputTokens: 10, outputTokens: 20, totalTokens: 30 }, | ||
content: [{ type: 'text', text: 'Third span here!' }], | ||
}), | ||
}), | ||
prompt: 'Where is the third span?', | ||
}); | ||
}); | ||
} | ||
|
||
run(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,10 +14,10 @@ import type { | |
import { normalize } from '@sentry/core'; | ||
import { createBasicSentryServer } from '@sentry-internal/test-utils'; | ||
import { execSync, spawn, spawnSync } from 'child_process'; | ||
import { existsSync, readFileSync, unlinkSync, writeFileSync } from 'fs'; | ||
import { join } from 'path'; | ||
import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'fs'; | ||
import { basename, join } from 'path'; | ||
import { inspect } from 'util'; | ||
import { afterAll, beforeAll, describe, test } from 'vitest'; | ||
import { afterAll, describe, test } from 'vitest'; | ||
import { | ||
assertEnvelopeHeader, | ||
assertSentryCheckIn, | ||
|
@@ -174,7 +174,10 @@ export function createEsmAndCjsTests( | |
testFn: typeof test | typeof test.fails, | ||
mode: 'esm' | 'cjs', | ||
) => void, | ||
options?: { failsOnCjs?: boolean; failsOnEsm?: boolean }, | ||
// `additionalDependencies` to install in a tmp dir for the esm and cjs tests | ||
// This could be used to override packages that live in the parent package.json for the specific run of the test | ||
// e.g. `{ ai: '^5.0.0' }` to test Vercel AI v5 | ||
options?: { failsOnCjs?: boolean; failsOnEsm?: boolean; additionalDependencies?: Record<string, string> }, | ||
RulaKhaled marked this conversation as resolved.
Show resolved
Hide resolved
|
||
): void { | ||
const mjsScenarioPath = join(cwd, scenarioPath); | ||
const mjsInstrumentPath = join(cwd, instrumentPath); | ||
|
@@ -187,36 +190,107 @@ export function createEsmAndCjsTests( | |
throw new Error(`Instrument file not found: ${mjsInstrumentPath}`); | ||
} | ||
|
||
const cjsScenarioPath = join(cwd, `tmp_${scenarioPath.replace('.mjs', '.cjs')}`); | ||
const cjsInstrumentPath = join(cwd, `tmp_${instrumentPath.replace('.mjs', '.cjs')}`); | ||
// Create a dedicated tmp directory that includes copied ESM & CJS scenario/instrument files. | ||
// If additionalDependencies are provided, we also create a nested package.json and install them there. | ||
const uniqueId = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 8)}`; | ||
const tmpDirPath = join(cwd, `tmp_${uniqueId}`); | ||
mkdirSync(tmpDirPath); | ||
|
||
// Copy ESM files as-is into tmp dir | ||
const esmScenarioBasename = basename(scenarioPath); | ||
const esmInstrumentBasename = basename(instrumentPath); | ||
const esmScenarioPathForRun = join(tmpDirPath, esmScenarioBasename); | ||
const esmInstrumentPathForRun = join(tmpDirPath, esmInstrumentBasename); | ||
writeFileSync(esmScenarioPathForRun, readFileSync(mjsScenarioPath, 'utf8')); | ||
writeFileSync(esmInstrumentPathForRun, readFileSync(mjsInstrumentPath, 'utf8')); | ||
|
||
// Pre-create CJS converted files inside tmp dir | ||
const cjsScenarioPath = join(tmpDirPath, esmScenarioBasename.replace('.mjs', '.cjs')); | ||
const cjsInstrumentPath = join(tmpDirPath, esmInstrumentBasename.replace('.mjs', '.cjs')); | ||
convertEsmFileToCjs(esmScenarioPathForRun, cjsScenarioPath); | ||
convertEsmFileToCjs(esmInstrumentPathForRun, cjsInstrumentPath); | ||
|
||
// Create a minimal package.json with requested dependencies (if any) and install them | ||
const additionalDependencies = options?.additionalDependencies ?? {}; | ||
if (Object.keys(additionalDependencies).length > 0) { | ||
const packageJson = { | ||
name: 'tmp-integration-test', | ||
private: true, | ||
version: '0.0.0', | ||
dependencies: additionalDependencies, | ||
} as const; | ||
|
||
writeFileSync(join(tmpDirPath, 'package.json'), JSON.stringify(packageJson, null, 2)); | ||
|
||
try { | ||
const deps = Object.entries(additionalDependencies).map(([name, range]) => { | ||
if (!range || typeof range !== 'string') { | ||
throw new Error(`Invalid version range for "${name}": ${String(range)}`); | ||
} | ||
return `${name}@${range}`; | ||
}); | ||
|
||
describe('esm', () => { | ||
const testFn = options?.failsOnEsm ? test.fails : test; | ||
callback(() => createRunner(mjsScenarioPath).withFlags('--import', mjsInstrumentPath), testFn, 'esm'); | ||
}); | ||
if (deps.length > 0) { | ||
// Prefer npm for temp installs to avoid Yarn engine strictness; see https://github.com/vercel/ai/issues/7777 | ||
// We rely on the generated package.json dependencies and run a plain install. | ||
const result = spawnSync('npm', ['install', '--silent', '--no-audit', '--no-fund'], { | ||
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. m: Should we run 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. not sure, I guess maybe npm is easier actually, just wondering... |
||
cwd: tmpDirPath, | ||
encoding: 'utf8', | ||
}); | ||
|
||
if (process.env.DEBUG) { | ||
// eslint-disable-next-line no-console | ||
console.log('[additionalDependencies via npm]', deps.join(' ')); | ||
// eslint-disable-next-line no-console | ||
console.log('[npm stdout]', result.stdout); | ||
// eslint-disable-next-line no-console | ||
console.log('[npm stderr]', result.stderr); | ||
} | ||
|
||
describe('cjs', () => { | ||
beforeAll(() => { | ||
// For the CJS runner, we create some temporary files... | ||
convertEsmFileToCjs(mjsScenarioPath, cjsScenarioPath); | ||
convertEsmFileToCjs(mjsInstrumentPath, cjsInstrumentPath); | ||
if (result.error) { | ||
throw new Error(`Failed to install additionalDependencies in tmp dir ${tmpDirPath}: ${result.error.message}`); | ||
} | ||
if (typeof result.status === 'number' && result.status !== 0) { | ||
throw new Error( | ||
`Failed to install additionalDependencies in tmp dir ${tmpDirPath} (exit ${result.status}):\n${ | ||
result.stderr || result.stdout || '(no output)' | ||
}`, | ||
); | ||
} | ||
} | ||
} catch (e) { | ||
// eslint-disable-next-line no-console | ||
console.error('Failed to install additionalDependencies:', e); | ||
throw e; | ||
} | ||
} | ||
|
||
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. Bug: Test Runner File Overwrite and Cleanup FailuresThe test runner has two distinct issues: First, if the |
||
describe('esm/cjs', () => { | ||
const esmTestFn = options?.failsOnEsm ? test.fails : test; | ||
describe('esm', () => { | ||
callback( | ||
() => createRunner(esmScenarioPathForRun).withFlags('--import', esmInstrumentPathForRun), | ||
esmTestFn, | ||
'esm', | ||
); | ||
}); | ||
|
||
const cjsTestFn = options?.failsOnCjs ? test.fails : test; | ||
describe('cjs', () => { | ||
callback(() => createRunner(cjsScenarioPath).withFlags('--require', cjsInstrumentPath), cjsTestFn, 'cjs'); | ||
}); | ||
|
||
// Clean up the tmp directory after both esm and cjs suites have run | ||
afterAll(() => { | ||
try { | ||
unlinkSync(cjsInstrumentPath); | ||
} catch { | ||
// Ignore errors here | ||
} | ||
try { | ||
unlinkSync(cjsScenarioPath); | ||
rmSync(tmpDirPath, { recursive: true, force: true }); | ||
} catch { | ||
// Ignore errors here | ||
if (process.env.DEBUG) { | ||
// eslint-disable-next-line no-console | ||
console.error(`Failed to remove tmp dir: ${tmpDirPath}`); | ||
} | ||
} | ||
}); | ||
|
||
const testFn = options?.failsOnCjs ? test.fails : test; | ||
callback(() => createRunner(cjsScenarioPath).withFlags('--require', cjsInstrumentPath), testFn, 'cjs'); | ||
}); | ||
} | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.