From 6016c8c7e3f37a037c46e73ef2e482cf7d6402d4 Mon Sep 17 00:00:00 2001 From: Mackenzie Zastrow Date: Mon, 8 Dec 2025 12:36:36 -0500 Subject: [PATCH] fix: Unify fixture loading and test skipping - Have a single `loadFixture` method usable from both the browser & node tests - Switch to skipping tests the same between Bedrock & OpenAI --- test/integ/__fixtures__/model-test-helpers.ts | 73 ++++++++++++++----- test/integ/__fixtures__/test-helpers.ts | 59 +++++---------- test/integ/agent.test.ts | 8 +- test/integ/bash.test.ts | 4 +- test/integ/bedrock.test.ts | 4 +- test/integ/browser/agent.browser.test.ts | 21 +----- test/integ/file-editor.test.ts | 4 +- test/integ/http-request.test.ts | 4 +- test/integ/notebook.test.ts | 4 +- test/integ/openai.test.ts | 3 +- 10 files changed, 89 insertions(+), 95 deletions(-) diff --git a/test/integ/__fixtures__/model-test-helpers.ts b/test/integ/__fixtures__/model-test-helpers.ts index 58826996..fad6b359 100644 --- a/test/integ/__fixtures__/model-test-helpers.ts +++ b/test/integ/__fixtures__/model-test-helpers.ts @@ -1,5 +1,24 @@ import { fromNodeProviderChain } from '@aws-sdk/credential-providers' -import type { Message, ContentBlock } from '$/sdk/types/messages.js' +import type { ContentBlock, Message } from '$/sdk/types/messages.js' + +/** + * Extracts plain text content from a Message object. + * + * This helper function handles different message formats by: + * - Extracting text from Message objects by filtering for textBlock content blocks + * - Joining multiple text blocks with newlines + * + * @param message - The message to extract text from. Message object with content blocks + * @returns The extracted text content as a string, or empty string if no content is found + */ +export const getMessageText = (message: Message): string => { + if (!message.content) return '' + + return message.content + .filter((block: ContentBlock) => block.type === 'textBlock') + .map((block) => block.text) + .join('\n') +} /** * Determines whether AWS integration tests should run based on environment and credentials. @@ -9,12 +28,12 @@ import type { Message, ContentBlock } from '$/sdk/types/messages.js' * * @returns Promise - true if tests should run, false if they should be skipped */ -export async function shouldRunTests(): Promise { +export async function shouldSkipBedrockTests(): Promise { // In a CI environment, we ALWAYS expect credentials to be configured. // A failure is better than a skip. if (process.env.CI) { console.log('✅ Running in CI environment, integration tests will run.') - return true + return false } // In a local environment, we check for credentials as a convenience. @@ -22,28 +41,46 @@ export async function shouldRunTests(): Promise { const credentialProvider = fromNodeProviderChain() await credentialProvider() console.log('✅ AWS credentials found locally, integration tests will run.') - return true + return false } catch { console.log('⏭️ AWS credentials not available locally, integration tests will be skipped.') - return false + return true } } /** - * Extracts plain text content from a Message object. + * Determines if OpenAI integration tests should be skipped. + * In CI environments, throws an error if API key is missing (tests should not be skipped). + * In local development, skips tests if API key is not available. * - * This helper function handles different message formats by: - * - Extracting text from Message objects by filtering for textBlock content blocks - * - Joining multiple text blocks with newlines - * - * @param message - The message to extract text from. Message object with content blocks - * @returns The extracted text content as a string, or empty string if no content is found + * @returns true if tests should be skipped, false if they should run + * @throws Error if running in CI and API key is missing */ -export const getMessageText = (message: Message): string => { - if (!message.content) return '' +export function shouldSkipOpenAITests(): boolean { + try { + const isCI = !!process.env.CI + const hasKey = !!process.env.OPENAI_API_KEY - return message.content - .filter((block: ContentBlock) => block.type === 'textBlock') - .map((block) => block.text) - .join('\n') + if (isCI && !hasKey) { + throw new Error('OpenAI API key must be available in CI environments') + } + + if (hasKey) { + if (isCI) { + console.log('✅ Running in CI environment with OpenAI API key - tests will run') + } else { + console.log('✅ OpenAI API key found for integration tests') + } + return false + } else { + console.log('⏭️ OpenAI API key not available - integration tests will be skipped') + return true + } + } catch (error) { + if (error instanceof Error && error.message.includes('CI environments')) { + throw error + } + console.log('⏭️ OpenAI API key not available - integration tests will be skipped') + return true + } } diff --git a/test/integ/__fixtures__/test-helpers.ts b/test/integ/__fixtures__/test-helpers.ts index e50b01af..478032b7 100644 --- a/test/integ/__fixtures__/test-helpers.ts +++ b/test/integ/__fixtures__/test-helpers.ts @@ -1,5 +1,9 @@ -import { readFileSync } from 'node:fs' -import { join } from 'node:path' +/** + * Checks whether we're running tests in the browser. + */ +export const isInBrowser = () => { + return globalThis?.process?.env == null +} /** * Helper to load fixture files from Vite URL imports. @@ -8,45 +12,16 @@ import { join } from 'node:path' * @param url - The URL from a Vite ?url import * @returns The file contents as a Uint8Array */ -export const loadFixture = (url: string): Uint8Array => { - const relativePath = url.startsWith('/') ? url.slice(1) : url - const filePath = join(process.cwd(), relativePath) - return new Uint8Array(readFileSync(filePath)) -} - -/** - * Determines if OpenAI integration tests should be skipped. - * In CI environments, throws an error if API key is missing (tests should not be skipped). - * In local development, skips tests if API key is not available. - * - * @returns true if tests should be skipped, false if they should run - * @throws Error if running in CI and API key is missing - */ -export const shouldSkipOpenAITests = (): boolean => { - try { - const isCI = !!process.env.CI - const hasKey = !!process.env.OPENAI_API_KEY - - if (isCI && !hasKey) { - throw new Error('OpenAI API key must be available in CI environments') - } - - if (hasKey) { - if (isCI) { - console.log('✅ Running in CI environment with OpenAI API key - tests will run') - } else { - console.log('✅ OpenAI API key found for integration tests') - } - return false - } else { - console.log('⏭️ OpenAI API key not available - integration tests will be skipped') - return true - } - } catch (error) { - if (error instanceof Error && error.message.includes('CI environments')) { - throw error - } - console.log('⏭️ OpenAI API key not available - integration tests will be skipped') - return true +export async function loadFixture(url: string): Promise { + if (isInBrowser()) { + const response = await globalThis.fetch(url) + const arrayBuffer = await response.arrayBuffer() + return new Uint8Array(arrayBuffer) + } else { + const { join } = await import('node:path') + const { readFile } = await import('node:fs/promises') + const relativePath = url.startsWith('/') ? url.slice(1) : url + const filePath = join(process.cwd(), relativePath) + return new Uint8Array(await readFile(filePath)) } } diff --git a/test/integ/agent.test.ts b/test/integ/agent.test.ts index 19c7a141..7b144a12 100644 --- a/test/integ/agent.test.ts +++ b/test/integ/agent.test.ts @@ -7,8 +7,8 @@ import { OpenAIModel } from '@strands-agents/sdk/openai' import { z } from 'zod' import { collectGenerator } from '$/sdk/__fixtures__/model-test-helpers.js' -import { shouldRunTests } from './__fixtures__/model-test-helpers.js' -import { loadFixture, shouldSkipOpenAITests } from './__fixtures__/test-helpers.js' +import { shouldSkipBedrockTests, shouldSkipOpenAITests } from './__fixtures__/model-test-helpers.js' +import { loadFixture } from './__fixtures__/test-helpers.js' // Import fixtures using Vite's ?url suffix import yellowPngUrl from './__resources__/yellow.png?url' @@ -37,7 +37,7 @@ const calculatorTool = tool({ const providers = [ { name: 'BedrockModel', - skip: !(await shouldRunTests()), + skip: await shouldSkipBedrockTests(), createModel: () => new BedrockModel(), }, { @@ -144,7 +144,7 @@ describe.each(providers)('Agent with $name', ({ name, skip, createModel }) => { }) // Create image block - const imageBytes = loadFixture(yellowPngUrl) + const imageBytes = await loadFixture(yellowPngUrl) const imageBlock = new ImageBlock({ format: 'png', source: { bytes: imageBytes }, diff --git a/test/integ/bash.test.ts b/test/integ/bash.test.ts index 73ba9ed8..e880a42f 100644 --- a/test/integ/bash.test.ts +++ b/test/integ/bash.test.ts @@ -1,9 +1,9 @@ import { describe, it, expect } from 'vitest' import { Agent, BedrockModel } from '$/sdk/index.js' import { bash } from '$/sdk/vended-tools/bash/index.js' -import { getMessageText, shouldRunTests } from './__fixtures__/model-test-helpers.js' +import { getMessageText, shouldSkipBedrockTests } from './__fixtures__/model-test-helpers.js' -describe.skipIf(!(await shouldRunTests()) || process.platform === 'win32')( +describe.skipIf((await shouldSkipBedrockTests()) || process.platform === 'win32')( 'Bash Tool Integration', { timeout: 60000 }, () => { diff --git a/test/integ/bedrock.test.ts b/test/integ/bedrock.test.ts index 95391137..10c51a46 100644 --- a/test/integ/bedrock.test.ts +++ b/test/integ/bedrock.test.ts @@ -9,9 +9,9 @@ import { } from '@strands-agents/sdk' import { collectIterator } from '$/sdk/__fixtures__/model-test-helpers.js' -import { shouldRunTests } from './__fixtures__/model-test-helpers.js' +import { shouldSkipBedrockTests } from './__fixtures__/model-test-helpers.js' -describe.skipIf(!(await shouldRunTests()))('BedrockModel Integration Tests', () => { +describe.skipIf(await shouldSkipBedrockTests())('BedrockModel Integration Tests', () => { describe('Streaming', () => { describe('Configuration', () => { it.concurrent('respects maxTokens configuration', async () => { diff --git a/test/integ/browser/agent.browser.test.ts b/test/integ/browser/agent.browser.test.ts index 88ccfa34..0789a88b 100644 --- a/test/integ/browser/agent.browser.test.ts +++ b/test/integ/browser/agent.browser.test.ts @@ -11,26 +11,7 @@ import { collectGenerator } from '$/sdk/__fixtures__/model-test-helpers.js' // Import fixtures import yellowPngUrl from '../__resources__/yellow.png?url' - -// Environment detection for browser vs Node.js -const isNode = typeof process !== 'undefined' && typeof process.versions !== 'undefined' && !!process.versions.node - -// Browser-compatible fixture loader -const loadFixture = async (url: string): Promise => { - if (isNode) { - // In Node.js, use synchronous file reading - const { readFileSync } = await import('node:fs') - const { join } = await import('node:path') - const relativePath = url.startsWith('/') ? url.slice(1) : url - const filePath = join(process.cwd(), relativePath) - return new Uint8Array(readFileSync(filePath)) - } else { - // In browser, use fetch API - const response = await globalThis.fetch(url) - const arrayBuffer = await response.arrayBuffer() - return new Uint8Array(arrayBuffer) - } -} +import { loadFixture } from '../__fixtures__/test-helpers.js' // Calculator tool for testing const calculatorTool = tool({ diff --git a/test/integ/file-editor.test.ts b/test/integ/file-editor.test.ts index ffa10c55..7d712b72 100644 --- a/test/integ/file-editor.test.ts +++ b/test/integ/file-editor.test.ts @@ -2,12 +2,12 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest' import { Agent, BedrockModel } from '$/sdk/index.js' import { fileEditor } from '$/sdk/vended-tools/file_editor/index.js' import { collectGenerator } from '$/sdk/__fixtures__/model-test-helpers.js' -import { shouldRunTests } from './__fixtures__/model-test-helpers.js' +import { shouldSkipBedrockTests } from './__fixtures__/model-test-helpers.js' import { promises as fs } from 'fs' import * as path from 'path' import { tmpdir } from 'os' -describe.skipIf(!(await shouldRunTests()))('FileEditor Tool Integration', () => { +describe.skipIf(await shouldSkipBedrockTests())('FileEditor Tool Integration', () => { let testDir: string // Shared agent configuration for all tests diff --git a/test/integ/http-request.test.ts b/test/integ/http-request.test.ts index 26bfaa63..65ab4b1f 100644 --- a/test/integ/http-request.test.ts +++ b/test/integ/http-request.test.ts @@ -1,9 +1,9 @@ import { describe, it, expect } from 'vitest' import { httpRequest } from '@strands-agents/sdk/vended_tools/http_request' import { Agent, BedrockModel } from '@strands-agents/sdk' -import { shouldRunTests } from './__fixtures__/model-test-helpers.js' +import { shouldSkipBedrockTests } from './__fixtures__/model-test-helpers.js' -describe.skipIf(!(await shouldRunTests()))('httpRequest tool (integration)', () => { +describe.skipIf(await shouldSkipBedrockTests())('httpRequest tool (integration)', () => { it('agent uses http_request tool to fetch weather from Open-Meteo', async () => { const agent = new Agent({ model: new BedrockModel({ maxTokens: 500 }), diff --git a/test/integ/notebook.test.ts b/test/integ/notebook.test.ts index c091487f..9c053918 100644 --- a/test/integ/notebook.test.ts +++ b/test/integ/notebook.test.ts @@ -3,9 +3,9 @@ import { Agent, BedrockModel } from '$/sdk/index.js' import type { AgentStreamEvent, AgentResult } from '$/sdk/index.js' import { notebook } from '$/sdk/vended-tools/notebook/index.js' import { collectGenerator } from '$/sdk/__fixtures__/model-test-helpers.js' -import { shouldRunTests } from './__fixtures__/model-test-helpers.js' +import { shouldSkipBedrockTests } from './__fixtures__/model-test-helpers.js' -describe.skipIf(!(await shouldRunTests()))('Notebook Tool Integration', () => { +describe.skipIf(await shouldSkipBedrockTests())('Notebook Tool Integration', () => { // Shared agent configuration for all tests const agentParams = { model: new BedrockModel({ diff --git a/test/integ/openai.test.ts b/test/integ/openai.test.ts index aed916ca..7efcebf8 100644 --- a/test/integ/openai.test.ts +++ b/test/integ/openai.test.ts @@ -4,7 +4,8 @@ import { Message } from '@strands-agents/sdk' import type { ToolSpec } from '@strands-agents/sdk' import { collectIterator } from '$/sdk/__fixtures__/model-test-helpers.js' -import { shouldSkipOpenAITests } from './__fixtures__/test-helpers.js' + +import { shouldSkipOpenAITests } from './__fixtures__/model-test-helpers.js' describe.skipIf(shouldSkipOpenAITests())('OpenAIModel Integration Tests', () => { describe('Configuration', () => {