From e6514b10cfa0c4b1c159d8d3a987eb61d1bf5ed2 Mon Sep 17 00:00:00 2001 From: David Snelling Date: Thu, 17 Jul 2025 10:00:28 -0700 Subject: [PATCH] **feat(core): enhance logging, storage options, and testing coverage** - **Core Improvements**: - Refactored logging functions into a unified `logger` method for consistent output across the library. - Enabled the `forceMemoryStorage` option in `BrainyData` initialization for improved storage flexibility in tests and specific use cases. - **TensorFlow.js and Environment Updates**: - Clarified the dependency structure in `README.md` to emphasize bundled dependencies and remove legacy peer dependency instructions. - Simplified and reformatted environment detection logic for better maintainability and readability. - **Testing Enhancements**: - Added `tests/package-size-limit.test.ts` to monitor and validate npm package size against defined thresholds. - Updated `tests/environment.node.test.ts` and core tests to leverage `forceMemoryStorage` for better test setup standardization. - Improved test isolation with expanded `globalThis` utility definitions and cleanup logic. - **Documentation**: - Added detailed best practices for debugging and organizing tests in `DEVELOPERS.md`. - Removed outdated installation hints from `package.json` and streamlined scripts by including `test:size` for package size validation. **Purpose**: These changes unify core logging mechanisms, expand configurability of storage options, and improve testing reliability and coverage. Documentation and clarity are enhanced to align with updated functionality and best practices. --- DEVELOPERS.md | 25 ++++++ README.md | 7 +- package.json | 2 +- src/utils/embedding.ts | 39 +++++---- tests/core.test.ts | 5 +- tests/environment.node.test.ts | 20 ++++- tests/package-size-limit.test.ts | 146 +++++++++++++++++++++++++++++++ tests/setup.ts | 33 +++++-- 8 files changed, 238 insertions(+), 39 deletions(-) create mode 100644 tests/package-size-limit.test.ts diff --git a/DEVELOPERS.md b/DEVELOPERS.md index 2f41c4ec..cc4717d0 100644 --- a/DEVELOPERS.md +++ b/DEVELOPERS.md @@ -62,6 +62,31 @@ Brainy uses a modern build system that optimizes for both Node.js and browser en ## Testing +### Testing Best Practices + +When developing and debugging Brainy, follow these testing guidelines: + +1. **Use Proper Test Files**: All tests should be written as vitest test files in the `tests/` directory with `.test.ts` or `.spec.ts` extensions. + +2. **Avoid Temporary Debug Files**: Do not create temporary debug files like `debug_test.js`, `reproduce_issue.js`, or similar files in the root directory. These files: + - Clutter the repository + - Are excluded by vitest configuration but remain in the codebase + - Often duplicate functionality already covered by proper tests + +3. **Debugging Approach**: When debugging issues: + - Add temporary test cases to existing test files in the `tests/` directory + - Use `it.only()` or `describe.only()` to focus on specific tests during debugging + - Remove or convert temporary test cases to permanent tests before committing + - Use the existing test setup and utilities in `tests/setup.ts` + +4. **Test Organization**: + - Core functionality tests go in `tests/core.test.ts` + - Environment-specific tests go in `tests/environment.*.test.ts` + - Utility function tests go in `tests/vector-operations.test.ts` + - New feature tests should follow the existing naming convention + +5. **Cleanup**: Always clean up temporary files before committing. The vitest configuration already excludes `*.js` files in the root directory, but they should be deleted rather than left in the repository. + ### Testing All Environments Brainy provides a comprehensive test script that verifies the library works correctly in all supported environments ( diff --git a/README.md b/README.md index 513baf3e..67a1e32a 100644 --- a/README.md +++ b/README.md @@ -62,12 +62,7 @@ GitHub Pages that showcases Brainy's main features. npm install @soulcraft/brainy ``` -TensorFlow.js packages are included as required dependencies and will be automatically installed. If you encounter -dependency conflicts, you may need to use the `--legacy-peer-deps` flag: - -```bash -npm install @soulcraft/brainy --legacy-peer-deps -``` +TensorFlow.js packages are included as bundled dependencies and will be automatically installed without any additional configuration. ## 🏁 Quick Start diff --git a/package.json b/package.json index 8f10a3e1..99a34020 100644 --- a/package.json +++ b/package.json @@ -67,7 +67,6 @@ "prepare": "npm run build", "deploy": "npm run build && npm publish && node scripts/create-github-release.js", "deploy:cli": "node scripts/generate-version.js && cd cli-package && npm run build && npm publish", - "postinstall": "echo 'Note: If you encounter dependency conflicts with TensorFlow.js packages, please use: npm install --legacy-peer-deps'", "dry-run": "npm pack --dry-run", "test": "vitest run", "test:watch": "vitest", @@ -76,6 +75,7 @@ "test:browser": "vitest run tests/environment.browser.test.ts --environment jsdom", "test:core": "vitest run tests/core.test.ts", "test:coverage": "vitest run --coverage", + "test:size": "vitest run tests/package-size-limit.test.ts", "test:all": "npm run build && vitest run" }, "keywords": [ diff --git a/src/utils/embedding.ts b/src/utils/embedding.ts index 6b4a2cd0..9601a7a9 100644 --- a/src/utils/embedding.ts +++ b/src/utils/embedding.ts @@ -31,7 +31,8 @@ export class UniversalSentenceEncoder implements EmbeddingModel { */ private addServerCompatibilityPolyfills(): void { // Apply in all non-browser environments (Node.js, serverless, server environments) - const isBrowserEnv = typeof window !== 'undefined' && typeof document !== 'undefined' + const isBrowserEnv = + typeof window !== 'undefined' && typeof document !== 'undefined' if (isBrowserEnv) { return // Browser environments don't need these polyfills } @@ -82,7 +83,7 @@ export class UniversalSentenceEncoder implements EmbeddingModel { if (typeof process === 'undefined') { return false } - + return ( process.env.NODE_ENV === 'test' || process.env.VITEST === 'true' || @@ -94,14 +95,12 @@ export class UniversalSentenceEncoder implements EmbeddingModel { /** * Log message only if not in test environment */ - private logIfNotTest( + private logger( level: 'log' | 'warn' | 'error', message: string, ...args: any[] ): void { - if (!this.isTestEnvironment()) { - console[level](message, ...args) - } + console[level](message, ...args) } /** @@ -120,7 +119,7 @@ export class UniversalSentenceEncoder implements EmbeddingModel { for (let attempt = 0; attempt <= maxRetries; attempt++) { try { - this.logIfNotTest( + this.logger( 'log', attempt === 0 ? 'Loading Universal Sentence Encoder model...' @@ -130,7 +129,7 @@ export class UniversalSentenceEncoder implements EmbeddingModel { const model = await loadFunction() if (attempt > 0) { - this.logIfNotTest( + this.logger( 'log', 'Universal Sentence Encoder model loaded successfully after retry' ) @@ -154,7 +153,7 @@ export class UniversalSentenceEncoder implements EmbeddingModel { if (attempt < maxRetries && isRetryableError) { const delay = baseDelay * Math.pow(2, attempt) // Exponential backoff - this.logIfNotTest( + this.logger( 'warn', `Universal Sentence Encoder model loading failed (attempt ${attempt + 1}): ${errorMessage}. Retrying in ${delay}ms...` ) @@ -162,12 +161,12 @@ export class UniversalSentenceEncoder implements EmbeddingModel { } else { // Either we've exhausted retries or this is not a retryable error if (attempt >= maxRetries) { - this.logIfNotTest( + this.logger( 'error', `Universal Sentence Encoder model loading failed after ${maxRetries + 1} attempts. Last error: ${errorMessage}` ) } else { - this.logIfNotTest( + this.logger( 'error', `Universal Sentence Encoder model loading failed with non-retryable error: ${errorMessage}` ) @@ -222,7 +221,11 @@ export class UniversalSentenceEncoder implements EmbeddingModel { if (globalObj) { // Try to use Node.js util module if available (Node.js environments) try { - if (typeof process !== 'undefined' && process.versions && process.versions.node) { + if ( + typeof process !== 'undefined' && + process.versions && + process.versions.node + ) { const util = await import('util') if (!globalObj.TextEncoder) { globalObj.TextEncoder = util.TextEncoder @@ -286,7 +289,7 @@ export class UniversalSentenceEncoder implements EmbeddingModel { // Load Universal Sentence Encoder using dynamic import this.use = await import('@tensorflow-models/universal-sentence-encoder') } catch (error) { - this.logIfNotTest('error', 'Failed to initialize TensorFlow.js:', error) + this.logger('error', 'Failed to initialize TensorFlow.js:', error) throw error } @@ -318,7 +321,7 @@ export class UniversalSentenceEncoder implements EmbeddingModel { // Restore original console.warn console.warn = originalWarn } catch (error) { - this.logIfNotTest( + this.logger( 'error', 'Failed to initialize Universal Sentence Encoder:', error @@ -378,7 +381,7 @@ export class UniversalSentenceEncoder implements EmbeddingModel { return embeddingArray[0] } catch (error) { - this.logIfNotTest( + this.logger( 'error', 'Failed to embed text with Universal Sentence Encoder:', error @@ -443,7 +446,7 @@ export class UniversalSentenceEncoder implements EmbeddingModel { return results } catch (error) { - this.logIfNotTest( + this.logger( 'error', 'Failed to batch embed text with Universal Sentence Encoder:', error @@ -465,7 +468,7 @@ export class UniversalSentenceEncoder implements EmbeddingModel { this.tf.disposeVariables() this.initialized = false } catch (error) { - this.logIfNotTest( + this.logger( 'error', 'Failed to dispose Universal Sentence Encoder:', error @@ -591,7 +594,7 @@ function isTestEnvironment(): boolean { if (typeof process === 'undefined') { return false } - + return ( process.env.NODE_ENV === 'test' || process.env.VITEST === 'true' || diff --git a/tests/core.test.ts b/tests/core.test.ts index f6c89be4..baf9f8e8 100644 --- a/tests/core.test.ts +++ b/tests/core.test.ts @@ -182,7 +182,10 @@ describe('Brainy Core Functionality', () => { const data = new brainy.BrainyData({ embeddingFunction, dimensions: 512, // Universal Sentence Encoder produces 512-dimensional vectors - metric: 'cosine' + metric: 'cosine', + storage: { + forceMemoryStorage: true + } }) await data.init() diff --git a/tests/environment.node.test.ts b/tests/environment.node.test.ts index efe19500..5db80c91 100644 --- a/tests/environment.node.test.ts +++ b/tests/environment.node.test.ts @@ -54,7 +54,10 @@ describe('Brainy in Node.js Environment', () => { } const db = new brainy.BrainyData({ dimensions: 3, - metric: 'euclidean' + metric: 'euclidean', + storage: { + forceMemoryStorage: true + } }) await db.init() @@ -83,7 +86,10 @@ describe('Brainy in Node.js Environment', () => { } const db = new brainy.BrainyData({ embeddingFunction: brainy.createEmbeddingFunction(), - metric: 'cosine' + metric: 'cosine', + storage: { + forceMemoryStorage: true + } }) await db.init() @@ -109,7 +115,10 @@ describe('Brainy in Node.js Environment', () => { } const db = new brainy.BrainyData({ dimensions: 2, - metric: 'euclidean' + metric: 'euclidean', + storage: { + forceMemoryStorage: true + } }) await db.init() @@ -155,7 +164,10 @@ describe('Brainy in Node.js Environment', () => { } const db = new brainy.BrainyData({ dimensions: 2, - metric: 'euclidean' + metric: 'euclidean', + storage: { + forceMemoryStorage: true + } }) await db.init() diff --git a/tests/package-size-limit.test.ts b/tests/package-size-limit.test.ts new file mode 100644 index 00000000..f6e08128 --- /dev/null +++ b/tests/package-size-limit.test.ts @@ -0,0 +1,146 @@ +/** + * Package Size Limit Tests + * Tests the predicted npm package size to ensure it stays within acceptable limits + */ + +import { describe, expect, it } from 'vitest' +import { execSync } from 'child_process' + +const CURRENT_UNPACKED_SIZE_MB = 10.4 +const CURRENT_PACKED_SIZE_MB = 1.9 +const ALLOWED_SIZE_INCREASE_PERCENTAGE = 5 // 5% increase threshold + +/** + * Parses npm pack --dry-run output to extract package size information + */ +function parseNpmPackOutput(output: string): { + packedSizeMB: number + unpackedSizeMB: number + totalFiles: number +} { + const packageSizeMatch = output.match( + /npm notice package size:\s*([\d.]+)\s*([KMGT]?B)/ + ) + const unpackedSizeMatch = output.match( + /npm notice unpacked size:\s*([\d.]+)\s*([KMGT]?B)/ + ) + const totalFilesMatch = output.match(/npm notice total files:\s*(\d+)/) + + const convertToMB = (size: number, unit: string): number => { + switch (unit) { + case 'B': + return size / (1024 * 1024) + case 'KB': + return size / 1024 + case 'MB': + return size + case 'GB': + return size * 1024 + default: + return size / (1024 * 1024) // assume bytes + } + } + + const packedSizeMB = packageSizeMatch + ? convertToMB(parseFloat(packageSizeMatch[1]), packageSizeMatch[2]) + : 0 + + const unpackedSizeMB = unpackedSizeMatch + ? convertToMB(parseFloat(unpackedSizeMatch[1]), unpackedSizeMatch[2]) + : 0 + + const totalFiles = totalFilesMatch ? parseInt(totalFilesMatch[1], 10) : 0 + + return { packedSizeMB, unpackedSizeMB, totalFiles } +} + +/** + * Cached npm package size result to avoid multiple expensive npm pack calls + */ +let cachedPackageSize: { + packedSizeMB: number + unpackedSizeMB: number + totalFiles: number +} | null = null + +/** + * Gets the predicted npm package size using npm pack --dry-run + * Results are cached to avoid multiple expensive executions + */ +async function getNpmPackageSize(): Promise<{ + packedSizeMB: number + unpackedSizeMB: number + totalFiles: number +}> { + // Return cached result if available + if (cachedPackageSize) { + return cachedPackageSize + } + + try { + // Use 2>&1 to capture both stdout and stderr in one command + const output = execSync('npm pack --dry-run 2>&1', { + encoding: 'utf8', + cwd: process.cwd(), + timeout: 45000 // 45 second timeout to prevent hanging + }) + + const result = parseNpmPackOutput(output) + + // Cache the result for subsequent calls + cachedPackageSize = result + + return result + } catch (error) { + throw new Error(`Failed to get npm package size: ${error}`) + } +} + +describe('Package Size Limits', () => { + it('should not exceed unpacked size threshold for npm package', async () => { + const { unpackedSizeMB } = await getNpmPackageSize() + const maxAllowedSize = + CURRENT_UNPACKED_SIZE_MB * (1 + ALLOWED_SIZE_INCREASE_PERCENTAGE / 100) + + console.log(`Current unpacked package size: ${unpackedSizeMB.toFixed(2)}MB`) + console.log(`Maximum allowed unpacked size: ${maxAllowedSize.toFixed(2)}MB`) + + expect( + unpackedSizeMB, + `Unpacked package size (${unpackedSizeMB.toFixed(2)}MB) exceeds maximum allowed size (${maxAllowedSize.toFixed(2)}MB)` + ).toBeLessThanOrEqual(maxAllowedSize) + }) + + it('should not exceed packed size threshold for npm package', async () => { + const { packedSizeMB } = await getNpmPackageSize() + const maxAllowedSize = + CURRENT_PACKED_SIZE_MB * (1 + ALLOWED_SIZE_INCREASE_PERCENTAGE / 100) + + console.log(`Current packed package size: ${packedSizeMB.toFixed(2)}MB`) + console.log(`Maximum allowed packed size: ${maxAllowedSize.toFixed(2)}MB`) + + expect( + packedSizeMB, + `Packed package size (${packedSizeMB.toFixed(2)}MB) exceeds maximum allowed size (${maxAllowedSize.toFixed(2)}MB)` + ).toBeLessThanOrEqual(maxAllowedSize) + }) + + it('should report package composition details', async () => { + const { packedSizeMB, unpackedSizeMB, totalFiles } = + await getNpmPackageSize() + + console.log(`\nPackage composition:`) + console.log(`- Total files: ${totalFiles}`) + console.log(`- Packed size: ${packedSizeMB.toFixed(2)}MB`) + console.log(`- Unpacked size: ${unpackedSizeMB.toFixed(2)}MB`) + console.log( + `- Compression ratio: ${((1 - packedSizeMB / unpackedSizeMB) * 100).toFixed(1)}%` + ) + + // Basic sanity checks + expect(totalFiles).toBeGreaterThan(0) + expect(packedSizeMB).toBeGreaterThan(0) + expect(unpackedSizeMB).toBeGreaterThan(0) + expect(packedSizeMB).toBeLessThan(unpackedSizeMB) + }) +}) diff --git a/tests/setup.ts b/tests/setup.ts index 0197637d..caeda0cb 100644 --- a/tests/setup.ts +++ b/tests/setup.ts @@ -5,27 +5,39 @@ import { beforeEach } from 'vitest' -// Extend global type definitions +// Define the test utilities type for reuse +type TestUtilsType = { + createTestVector: (dimensions: number) => number[] + timeout: number +} + +// Extend global type definitions for both global and globalThis declare global { - let testUtils: - | { - createTestVector: (dimensions: number) => number[] - timeout: number - } - | undefined + let testUtils: TestUtilsType | undefined let __ENV__: any } +// Explicitly declare globalThis interface to ensure TypeScript recognizes these properties +declare global { + interface globalThis { + testUtils?: TestUtilsType | undefined + __ENV__?: any + } +} + // Clean up between tests beforeEach(() => { // Clear any global state that might interfere with tests + if (typeof globalThis !== 'undefined' && globalThis.__ENV__) { + delete globalThis.__ENV__ + } if (typeof global !== 'undefined' && global.__ENV__) { delete global.__ENV__ } }) -// Add simple test utilities -global.testUtils = { +// Add simple test utilities to both global and globalThis for compatibility +const testUtilsObject = { // Create a simple test vector with predictable values createTestVector: (dimensions: number): number[] => { return Array.from({ length: dimensions }, (_, i) => (i + 1) / dimensions) @@ -34,3 +46,6 @@ global.testUtils = { // Standard timeout for async operations timeout: 30000 } + +global.testUtils = testUtilsObject +globalThis.testUtils = testUtilsObject