test: add simple e2e tests for AI SDK#75
Conversation
- add check installation test
There was a problem hiding this comment.
Pull request overview
This PR adds end-to-end testing infrastructure for the AI SDK with a focus on verifying basic tool functionality. The changes introduce a new test package within the pnpm workspace to validate checkConnection and checkInstallation tools using the Vercel AI SDK, along with documentation updates and code formatting improvements.
- Add a new
testdirectory with e2e tests for AI SDK tools - Configure pnpm workspace to include the test package for monorepo testing
- Document testing workflow in README with setup and local development instructions
- Clean up vite.config.ts formatting for consistency (single quotes, cleaner structure)
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| test/package.json | Defines test package with dependencies and test scripts (though some scripts reference missing files) |
| test/tsconfig.json | TypeScript configuration for the test package |
| test/ai-e2e-simple.ts | E2E test script to verify checkConnection and checkInstallation tools with OpenAI |
| pnpm-workspace.yaml | Adds test directory to workspace for monorepo integration |
| pnpm-lock.yaml | Adds dependency entries for the new test package |
| README.md | Documents testing setup, prerequisites, and workflow for running e2e tests |
| sdk/vite.config.ts | Formatting cleanup (single quotes, simplified define property) |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| console.log(); | ||
|
|
||
| // comment out the following code to test the tool call (will use OpenAI credits) | ||
| // Test 1: Check connection - simple string parameter |
There was a problem hiding this comment.
Duplicate comment. The comment "Test 1: Check connection - simple string parameter" appears both on line 32 (as a standalone comment) and line 35 (in the code). Remove the duplicate on line 32.
| // Test 1: Check connection - simple string parameter |
| - OpenAI API key (required for AI model) | ||
| - Ampersand API credentials | ||
|
|
||
| 1. Configure environment variables in `test/.env`: |
There was a problem hiding this comment.
Missing .env.example file. The README and PR description mention cp .env.example .env but no .env.example file was added to the test directory. This file should be included to help developers set up the required environment variables.
| "test": "tsx ai-e2e-vercel-ai.ts", | ||
| "test:simple": "tsx ai-e2e-simple.ts", | ||
| "test:ts-node": "ts-node ai-e2e-vercel-ai.ts" |
There was a problem hiding this comment.
The package.json references a test file ai-e2e-vercel-ai.ts on lines 7 and 9, but this file does not exist in the test directory. Either add the missing file or remove these script entries.
| "test": "tsx ai-e2e-vercel-ai.ts", | |
| "test:simple": "tsx ai-e2e-simple.ts", | |
| "test:ts-node": "ts-node ai-e2e-vercel-ai.ts" | |
| "test:simple": "tsx ai-e2e-simple.ts" |
| "checkJs": false, | ||
| "types": ["node"] | ||
| }, | ||
| "include": ["./**/*"], |
There was a problem hiding this comment.
[nitpick] The tsconfig.json has a broad include pattern ["./**/*"] which will include all files in the test directory, including potentially unwanted files like package-lock.json. Consider being more specific, e.g., ["**/*.ts"] to only include TypeScript files.
| "include": ["./**/*"], | |
| "include": ["**/*.ts", "**/*.js"], |
| checkInstallationResult.steps[0].toolCalls?.[0]?.toolName, | ||
| ); | ||
| console.log( | ||
| '- Tool args:', | ||
| JSON.stringify(checkInstallationResult.steps[0].toolCalls?.[0]?.args), |
There was a problem hiding this comment.
Potential runtime error when accessing array index without checking if array exists or has elements. If checkInstallationResult.steps is empty or undefined, accessing steps[0] will cause a runtime error. Add a check before accessing the array.
| checkInstallationResult.steps[0].toolCalls?.[0]?.toolName, | |
| ); | |
| console.log( | |
| '- Tool args:', | |
| JSON.stringify(checkInstallationResult.steps[0].toolCalls?.[0]?.args), | |
| Array.isArray(checkInstallationResult.steps) && checkInstallationResult.steps.length > 0 | |
| ? checkInstallationResult.steps[0].toolCalls?.[0]?.toolName | |
| : 'No steps available', | |
| ); | |
| console.log( | |
| '- Tool args:', | |
| Array.isArray(checkInstallationResult.steps) && checkInstallationResult.steps.length > 0 | |
| ? JSON.stringify(checkInstallationResult.steps[0].toolCalls?.[0]?.args) | |
| : 'No steps available', |
| async function testSimpleToolCall() { | ||
| console.log('Starting simple tool call test...\n'); | ||
| console.log('Environment variables:'); | ||
| console.log( | ||
| '- AMPERSAND_API_KEY:', | ||
| process.env.AMPERSAND_API_KEY ? 'SET' : 'NOT SET', | ||
| ); | ||
| console.log('- AMPERSAND_PROJECT_ID:', process.env.AMPERSAND_PROJECT_ID); | ||
| console.log('- AMPERSAND_GROUP_REF:', process.env.AMPERSAND_GROUP_REF); | ||
| console.log(); | ||
|
|
||
| // comment out the following code to test the tool call (will use OpenAI credits) | ||
| // Test 1: Check connection - simple string parameter | ||
|
|
||
| try { | ||
| // Test 1: Check connection - simple string parameter | ||
| console.log('Test 1: Checking connection for Salesforce...'); | ||
| const checkResult = await generateText({ | ||
| model: openai('gpt-4o-mini'), | ||
| tools: { | ||
| checkConnection: checkConnectionTool, | ||
| }, | ||
| maxSteps: 5, | ||
| prompt: 'Check if there is an active connection for salesforce', | ||
| }); | ||
|
|
||
| console.log('Check Connection Result:'); | ||
| console.log('- Text:', checkResult.text); | ||
| console.log('- Tool Calls:', JSON.stringify(checkResult.steps, null, 2)); | ||
| console.log('\n'); | ||
|
|
||
| console.log('Test completed successfully!'); | ||
| } catch (error) { | ||
| console.error('Error during test execution:', error); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| // Test 2: Check installation - simple string parameter | ||
| try { | ||
| console.log('Test 2: Checking installation for Salesforce...'); | ||
| const checkInstallationResult = await generateText({ | ||
| model: openai('gpt-4o-mini'), | ||
| tools: { | ||
| checkInstallation: checkInstallationTool, | ||
| }, | ||
| maxSteps: 5, | ||
| prompt: 'Check if there is an active installation for salesforce', | ||
| }); | ||
|
|
||
| console.log('Check Installation Result:'); | ||
| console.log('- Text:', checkInstallationResult.text); | ||
| console.log( | ||
| '- Tool called:', | ||
| checkInstallationResult.steps[0].toolCalls?.[0]?.toolName, | ||
| ); | ||
| console.log( | ||
| '- Tool args:', | ||
| JSON.stringify(checkInstallationResult.steps[0].toolCalls?.[0]?.args), | ||
| ); | ||
| console.log('\n'); | ||
|
|
||
| console.log('Installation test completed successfully!'); | ||
| } catch (error) { | ||
| console.error('Error during installation test execution:', error); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
The test lacks assertions and doesn't verify the actual behavior. The test only logs outputs and doesn't validate that the tools work correctly (e.g., checking if checkResult.text contains expected values, or if found is true/false as expected). Consider adding assertions or converting this to a proper automated test with a testing framework.
|
|
||
| # Or from the test directory | ||
| cd test | ||
| npm run test:simple |
There was a problem hiding this comment.
[nitpick] Inconsistent command examples. Line 82 uses pnpm --filter ai-e2e-test test:simple but line 86 uses npm run test:simple. Since this is a pnpm workspace, it's better to be consistent and use pnpm commands. Consider changing line 86 to pnpm test:simple instead of npm run test:simple.
| npm run test:simple | |
| pnpm test:simple |
| * It uses the checkConnection and checkInstallation tools to check the connection and installation. | ||
| * It uses the generateText function to generate the text response. | ||
| * | ||
| * note: this test will use OpenAI credits |
There was a problem hiding this comment.
[nitpick] Inconsistent capitalization in comments. The docstring on line 17 says "note: this test will use OpenAI credits" with lowercase "note". Consider capitalizing "Note:" to match standard documentation style.
| * note: this test will use OpenAI credits | |
| * Note: this test will use OpenAI credits |
| console.log('- AMPERSAND_GROUP_REF:', process.env.AMPERSAND_GROUP_REF); | ||
| console.log(); | ||
|
|
||
| // comment out the following code to test the tool call (will use OpenAI credits) |
There was a problem hiding this comment.
The comment is confusing and appears to be misplaced. It says "comment out the following code to test the tool call" but the code is not commented out. Consider either removing this comment or clarifying the instruction (e.g., "Uncomment the following code to skip test 1" if that's the intent).
| // comment out the following code to test the tool call (will use OpenAI credits) | |
| // To skip Test 1 and save OpenAI credits, comment out the following block. |
|
|
||
| export default defineConfig({ | ||
| define: { | ||
| 'process.env': { |
There was a problem hiding this comment.
strips out the the other variables like the API key
- Remove npm package-lock.json from test directory - Update README.md test command to use pnpm - Aligns test folder with monorepo's pnpm package manager 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Extract environment variable checking into dedicated function - Add validation to throw errors if required variables are missing - Improves test reliability by failing fast when env vars are not set 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
caiopizzol
left a comment
There was a problem hiding this comment.
Overall it looks good!
A few minor tweaks
| console.log('- Text:', checkInstallationResult.text); | ||
| console.log( | ||
| '- Tool called:', | ||
| checkInstallationResult.steps[0].toolCalls?.[0]?.toolName, |
There was a problem hiding this comment.
steps[0] might not exist if the model responds with text before calling the tool (or doesn't call it at all). This will crash with "Cannot read properties of undefined".
Consider using steps?.[0]?.toolCalls?.[0] or iterating to find the step with tool calls.
| ); | ||
| console.log( | ||
| '- Tool args:', | ||
| JSON.stringify(checkInstallationResult.steps[0].toolCalls?.[0]?.args), |
| } | ||
|
|
||
| // Run the test | ||
| testSimpleToolCall(); |
There was a problem hiding this comment.
The async function is called without await or .catch(), so if checkEnvironmentVariables() throws, it becomes an unhandled promise rejection instead of hitting your error handling.
| "scripts": { | ||
| "test": "tsx ai-e2e-vercel-ai.ts", | ||
| "test:simple": "tsx ai-e2e-simple.ts", | ||
| "test:ts-node": "ts-node ai-e2e-vercel-ai.ts" |
There was a problem hiding this comment.
The test and test:ts-node scripts reference ai-e2e-vercel-ai.ts but that file isn't in the branch. Should it be added or should these scripts be removed?
| console.log('Test completed successfully!'); | ||
| } catch (error) { | ||
| console.error('Error during test execution:', error); | ||
| process.exit(1); |
There was a problem hiding this comment.
If Test 1 fails, process.exit(1) prevents Test 2 from running. Might be intentional, but if these tests are independent it could be useful to run both and report all failures at the end.
Summary
test:simplescript to verifycheckConnectionandcheckInstallationtoolsTesting Steps
Configure test environment variables:
Build the SDK and run tests:
Verify test output shows successful connection checks for Salesforce