Unify integ tests to run in browser & node

Rather than special casing the browser, run *all* integ tests in both browser & node by default, opting out when we need tests to run in only a single environment.

For opting out, we now have two options:

 - For one off tests, use `skip: isInBrowser()`
 - For entire tests, use the file convention `thing.node.test.ts` and that will exclude it from browser tests

This refactoring needed a bit of plumbing to make tests accommodate both environments, but I think it worked pretty well

Author: zastrowm

.github/workflows/integration-test.yml

@@ -86,11 +86,11 @@ jobs:
       - name: Run integration tests
         run: npm run test:integ:all
 
-      - name: Upload browser test screenshots
+      - name: Upload Artifacts
         if: always()
         uses: actions/upload-artifact@v5
         with:
-          name: browser-test-screenshots
-          path: tests_integ/browser/__screenshots__/
+          name: test-artifacts
+          path: ./test/.artifacts/
           retention-days: 4
           if-no-files-found: ignore

.github/workflows/test-lint.yml

@@ -53,4 +53,13 @@ jobs:
         run: npm run build
 
       - name: Test packaging
-        run: npm run test:package
+        run: npm run test:package
+
+      - name: Upload Artifacts
+        if: always()
+        uses: actions/upload-artifact@v5
+        with:
+          name: test-artifacts
+          path: ./test/.artifacts/
+          retention-days: 4
+          if-no-files-found: ignore

.gitignore

@@ -37,4 +37,6 @@ Thumbs.db
 .env.production.local
 
 # Github workflow artifacts
-.artifact
+.artifact
+# Test artifacts
+test/.artifacts

package-lock.json

@@ -33,6 +33,10 @@
     "./vended_tools/http_request": {
       "types": "./dist/vended_tools/http_request/index.d.ts",
       "default": "./dist/vended_tools/http_request/index.js"
+    },
+    "./vended_tools/bash": {
+      "types": "./dist/vended_tools/bash/index.d.ts",
+      "default": "./dist/vended_tools/bash/index.js"
     }
   },
   "scripts": {
@@ -78,6 +82,7 @@
     "@vitest/browser": "^4.0.15",
     "@vitest/browser-playwright": "^4.0.15",
     "@vitest/coverage-v8": "^4.0.15",
+    "@vitest/ui": "^4.0.15",
     "eslint": "^9.0.0",
     "eslint-plugin-tsdoc": "^0.5.0",
     "husky": "^9.1.7",

package.json

@@ -265,7 +265,7 @@ export class OpenAIModel extends Model<OpenAIModelConfig> {
       // In Node.js, can use OPENAI_API_KEY environment variable as fallback
       const hasEnvKey =
         typeof process !== 'undefined' && typeof process.env !== 'undefined' && process.env.OPENAI_API_KEY
-      if (!apiKey && !hasEnvKey) {
+      if (!apiKey && !hasEnvKey && !clientConfig?.apiKey) {
         throw new Error(
           "OpenAI API key is required. Provide it via the 'apiKey' option or set the OPENAI_API_KEY environment variable."
         )

src/models/openai.ts

@@ -1,5 +1,10 @@
-import { fromNodeProviderChain } from '@aws-sdk/credential-providers'
-import type { Message, ContentBlock } from '../../src/types/messages.js'
+import { BedrockModel, type BedrockModelOptions } from '@strands-agents/sdk'
+import { OpenAIModel, type OpenAIModelOptions } from '@strands-agents/sdk/openai'
+
+import type { Message, ContentBlock } from '@strands-agents/sdk'
+import { isInBrowser } from './test-helpers.js'
+
+export * from '../../src/__fixtures__/model-test-helpers.js'
 
 /**
  * Determines whether AWS integration tests should run based on environment and credentials.
@@ -9,22 +14,71 @@ import type { Message, ContentBlock } from '../../src/types/messages.js'
  *
  * @returns Promise<boolean> - true if tests should run, false if they should be skipped
  */
-export async function shouldRunTests(): Promise<boolean> {
+export async function shouldSkipBedrockTests(): Promise<boolean> {
   // In a CI environment, we ALWAYS expect credentials to be configured.
   // A failure is better than a skip.
-  if (process.env.CI) {
+  if (globalThis.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.
   try {
-    const credentialProvider = fromNodeProviderChain()
-    await credentialProvider()
-    console.log('✅ AWS credentials found locally, integration tests will run.')
-    return true
+    if (isInBrowser()) {
+      const { commands } = await import('vitest/browser')
+      await commands.getAwsCredentials()
+      console.log('✅ credentials found via vitest, integration tests will run.')
+      return false
+    } else {
+      const { fromNodeProviderChain } = await import('@aws-sdk/credential-providers')
+      const credentialProvider = fromNodeProviderChain()
+      await credentialProvider()
+      console.log('✅ AWS credentials found locally, integration tests will run.')
+      return false
+    }
   } catch {
-    console.log('⏭️ AWS credentials not available locally, integration tests will be skipped.')
+    console.log('⏭️ AWS credentials not available, integration tests will be skipped.')
+    return true
+  }
+}
+
+/**
+ * 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 = async (): Promise<boolean> => {
+  const getApiKey = async (): Promise<string> => {
+    if (isInBrowser()) {
+      const { commands } = await import('vitest/browser')
+      return await commands.getOpenAIAPIKey()
+    } else {
+      return globalThis.process.env.OPENAI_API_KEY as string
+    }
+  }
+
+  // In a CI environment, we ALWAYS expect credentials to be configured.
+  // A failure is better than a skip.
+  if (globalThis.process?.env?.CI) {
+    console.log('✅ Running in CI environment, integration tests will run.')
+    const apiKey = await getApiKey()
+
+    if (!apiKey) {
+      throw new Error('OpenAI API key must be available in CI environments')
+    }
+
+    return false
+  }
+
+  const apiKey = await getApiKey()
+  if (!apiKey) {
+    console.log('⏭️  OpenAI API key not available - integration tests will be skipped')
+    return true
+  } else {
+    console.log('⏭️  OpenAI API key available - integration tests will run')
     return false
   }
 }
@@ -47,3 +101,38 @@ export const getMessageText = (message: Message): string => {
     .map((block) => block.text)
     .join('\n')
 }
+
+export function createBedrockModel(options: BedrockModelOptions = {}) {
+  if (isInBrowser()) {
+    return new BedrockModel({
+      ...options,
+      clientConfig: {
+        ...(options.clientConfig ?? {}),
+        credentials: async () => {
+          const { commands } = await import('vitest/browser')
+          return await commands.getAwsCredentials()
+        },
+      },
+    })
+  } else {
+    return new BedrockModel(options)
+  }
+}
+
+export function createOpenAIModel(config: OpenAIModelOptions = {}) {
+  if (isInBrowser()) {
+    return new OpenAIModel({
+      ...config,
+      clientConfig: {
+        ...(config.clientConfig ?? {}),
+        apiKey: async (): Promise<string> => {
+          const { commands } = await import('vitest/browser')
+          return await commands.getOpenAIAPIKey()
+        },
+        dangerouslyAllowBrowser: true,
+      },
+    })
+  } else {
+    return new OpenAIModel()
+  }
+}

tests_integ/__fixtures__/model-test-helpers.ts

@@ -1,5 +1,6 @@
-import { readFileSync } from 'node:fs'
-import { join } from 'node:path'
+export const isInBrowser = () => {
+  return globalThis?.process?.env == null
+}
 
 /**
  * Helper to load fixture files from Vite URL imports.
@@ -8,45 +9,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 const loadFixture = async (url: string): Promise<Uint8Array> => {
+  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))
   }
 }