-
-
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 1 commit
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,7 @@ export function createEsmAndCjsTests( | |
testFn: typeof test | typeof test.fails, | ||
mode: 'esm' | 'cjs', | ||
) => void, | ||
options?: { failsOnCjs?: boolean; failsOnEsm?: boolean }, | ||
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,31 +187,97 @@ 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}`; | ||
}); | ||
|
||
if (deps.length > 0) { | ||
// --ignore-engines is needed to avoid engine mismatches when installing deps in the tmp dir | ||
// (e.g. Vercel AI v5 requires a package that requires Node >= 20 while the system Node is 18) | ||
// https://github.com/vercel/ai/issues/7777 | ||
const result = spawnSync('yarn', ['add', '--non-interactive', '--ignore-engines', ...deps], { | ||
RulaKhaled marked this conversation as resolved.
Show resolved
Hide resolved
|
||
cwd: tmpDirPath, | ||
encoding: 'utf8', | ||
}); | ||
|
||
if (process.env.DEBUG) { | ||
// eslint-disable-next-line no-console | ||
console.log('[additionalDependencies]', deps.join(' ')); | ||
// eslint-disable-next-line no-console | ||
console.log('[yarn stdout]', result.stdout); | ||
// eslint-disable-next-line no-console | ||
console.log('[yarn stderr]', result.stderr); | ||
} | ||
|
||
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', () => { | ||
const testFn = options?.failsOnEsm ? test.fails : test; | ||
callback(() => createRunner(mjsScenarioPath).withFlags('--import', mjsInstrumentPath), testFn, 'esm'); | ||
callback(() => createRunner(esmScenarioPathForRun).withFlags('--import', esmInstrumentPathForRun), testFn, 'esm'); | ||
}); | ||
|
||
describe('cjs', () => { | ||
beforeAll(() => { | ||
// For the CJS runner, we create some temporary files... | ||
convertEsmFileToCjs(mjsScenarioPath, cjsScenarioPath); | ||
convertEsmFileToCjs(mjsInstrumentPath, cjsInstrumentPath); | ||
}); | ||
|
||
// Clean up the tmp directory once CJS tests are finished | ||
afterAll(() => { | ||
try { | ||
unlinkSync(cjsInstrumentPath); | ||
rmSync(tmpDirPath, { recursive: true, force: true }); | ||
} catch { | ||
// Ignore errors here | ||
} | ||
try { | ||
unlinkSync(cjsScenarioPath); | ||
} catch { | ||
// Ignore errors here | ||
if (process.env.DEBUG) { | ||
// eslint-disable-next-line no-console | ||
console.error(`Failed to remove tmp dir: ${tmpDirPath}`); | ||
} | ||
RulaKhaled marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
}); | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.