test(node): Enable additionalDependencies in integration runner #17361
Conversation
| const cjsInstrumentPath = join(cwd, `tmp_${instrumentPath.replace('.mjs', '.cjs')}`); | ||
| // If additionalDependencies are provided, we create a dedicated tmp directory that includes | ||
| // copied ESM & CJS scenario/instrument files and a nested package.json with those dependencies installed. | ||
| const useTmpDir = Boolean(options?.additionalDependencies && Object.keys(options.additionalDependencies).length > 0); |
There was a problem hiding this comment.
Hmm, what about simply always creating a tmp dir? Would unify this a bit, I suppose...?
RulaKhaled
changed the title
RulaKhaled
force-pushed
the
support-versioning-in-runner
branch
from
4da02bd to
8c6f644
Compare
RulaKhaled
changed the title
| 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.
m: Should we run yarn add here instead? 🤔
There was a problem hiding this comment.
not sure, I guess maybe npm is easier actually, just wondering...
|
Side note, maybe for a follow up: we seem to duplicate this in node-core-integration-tests as well 🤔 we should probably extract the runner into e.g. test-utils and re-use it everywhere we need it. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| throw e; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Bug: Test Runner File Overwrite and Cleanup Failures
The test runner has two distinct issues: First, if the instrumentPath does not end with .mjs, the CJS conversion overwrites the ESM instrument file. This occurs because the CJS filename generation logic fails to create a distinct path when the original file lacks the .mjs extension, causing both ESM and CJS versions to point to the same temporary file. Second, temporary directories created for tests are not reliably cleaned up. If npm install fails during setup, the afterAll cleanup hook, defined within a describe block, will not execute, leaving the temporary directory behind.
This update enhances the Node integration test runner to support per-scenario dependency overrides via a temporary folder that contains package.json.
When additionalDependencies are provided, the runner now:
ESM and CJS test modes continue to run normally using the files from the temp workspace.
Also adds: