feat(PINE-30): E2E namespace operations tests - #28
Merged
Conversation
Implemented comprehensive E2E tests for connection management: Pinecone Tests: - Connection modal display and form validation - Required field validation - Error handling for invalid API keys - Successful connection with valid credentials - Collections/indexes display after connection - Disconnect functionality - Reconnection after disconnect - Profile saving and loading - Unreachable URL error handling Qdrant/Weaviate Tests: - Skeleton tests with test.skip() and TODO comments - Will be enabled when adapter system is integrated Test Features: - Uses Docker test containers from docker-compose.test.yml - Leverages existing e2e/electron.setup.ts helpers - Auto-skips tests requiring real API keys when not available - Comprehensive error path testing Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add comprehensive E2E tests for index/collection operations: - List indexes/collections after connecting - View index/collection stats (vector count, dimensions) - Create new collection with provider-specific settings - Delete collection (with confirmation) - Refresh collection list Implementation: - 8 active Pinecone tests covering full index lifecycle - 13 skipped Qdrant/Weaviate tests with TODO comments - Uses existing e2e/ infrastructure from PINE-28 - Tests use window.electronAPI for IPC communication - Proper test isolation with unique profile/index names Test coverage: ✅ Pinecone: List, stats, create, delete, refresh, error handling 🔲 Qdrant: Pending adapter integration 🔲 Weaviate: Pending adapter integration References: PINE-29 Related: IndexesPanel.tsx, IndexConfigView.tsx Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
11 comprehensive tests for Pinecone namespace functionality: - List namespaces in an index - Select namespace to view vectors - Clone namespace with progress tracking - Duplicate namespace with progress events - Namespace stats display - Cancellation support - Error handling Test features: - Progress event validation - Resource cleanup - Non-destructive (uses temp resources) - Requires PINECONE_API_KEY for execution Closes PINE-30
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
📝 WalkthroughWalkthroughThis PR adds comprehensive end-to-end test suites for an Electron app supporting Pinecone, Qdrant, and Weaviate integrations. Four new Playwright test files validate connection flows, index/collection management, and namespace operations through IPC communication, with Pinecone fully tested and Qdrant/Weaviate scaffolded. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@e2e/connection-flow.spec.ts`:
- Around line 175-216: Add the missing disconnect declaration to the ElectronAPI
type so the TS interface matches the runtime: open the ElectronAPI.pinecone
interface and add a method signature for disconnect that accepts a profileId:
string and returns Promise<void> (i.e., declare disconnect: (profileId: string)
=> Promise<void> on the pinecone object) to ensure the preload/main implemented
method is typed.
In `@e2e/index-collection-management.spec.ts`:
- Around line 155-198: The test passes createParams with serverlessSpec which
doesn't match the expected CreateIndexParams shape; update createParams used in
the test (the object passed to electronAPI.pinecone.createIndex) so it nests the
cloud/region under params.spec.serverless (e.g. params.spec = { serverless: {
cloud, region } }) instead of serverlessSpec, ensuring the call to
window.electronAPI.pinecone.createIndex receives the correct spec structure that
pinecone-service.ts expects when reading params.spec.
🧹 Nitpick comments (1)
e2e/namespace-operations.spec.ts (1)
131-198: Clean up the test namespace (or use a dedicated test index).
This test writes vectors into an existing index and leaves namespaces behind, which can pollute real data and accumulate costs. Consider deleting the test data or isolating everything in a temporary index that’s removed in teardown.
Comment on lines
+175
to
+216
| test('should handle disconnect functionality', async () => { | ||
| const { page } = electronContext | ||
|
|
||
| // Check if real API key is available | ||
| const hasRealApiKey = !!process.env.PINECONE_API_KEY && | ||
| process.env.PINECONE_API_KEY !== 'dummy-key-for-local-testing' | ||
|
|
||
| if (!hasRealApiKey) { | ||
| test.skip() | ||
| return | ||
| } | ||
|
|
||
| // Create and connect to a profile | ||
| const profileId = await createPineconeTestProfile( | ||
| page, | ||
| 'Disconnect Test', | ||
| process.env.PINECONE_API_KEY | ||
| ) | ||
|
|
||
| await page.evaluate(async (id) => { | ||
| const profiles = await (window as any).electronAPI.profiles.getAll() | ||
| const profile = profiles.find((p: any) => p.id === id) | ||
| if (profile) { | ||
| await (window as any).electronAPI.pinecone.connect(id, profile) | ||
| } | ||
| }, profileId) | ||
|
|
||
| await page.waitForTimeout(1000) | ||
|
|
||
| // Disconnect from the profile | ||
| const disconnectSuccess = await page.evaluate(async (id) => { | ||
| try { | ||
| await (window as any).electronAPI.pinecone.disconnect(id) | ||
| return true | ||
| } catch (error) { | ||
| console.error('Disconnect failed:', error) | ||
| return false | ||
| } | ||
| }, profileId) | ||
|
|
||
| expect(disconnectSuccess).toBe(true) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for disconnect in pinecone context
rg -n "disconnect" -i --type ts --type js | grep -i pineconeRepository: stepandel/pinecone-explorer
Length of output: 1182
🏁 Script executed:
# Search for pinecone API definitions and implementations
fd -e ts -e js | xargs rg -l "pinecone" | head -20Repository: stepandel/pinecone-explorer
Length of output: 556
🏁 Script executed:
# Look for preload script and electron main process files
fd -e ts -e js -path "*preload*" -o -path "*main*" | grep -E "(preload|main)" | head -20Repository: stepandel/pinecone-explorer
Length of output: 239
🏁 Script executed:
# Check the electron type definitions to see if disconnect is exposed
cat -n src/types/electron.d.tsRepository: stepandel/pinecone-explorer
Length of output: 16198
🏁 Script executed:
# Check the preload script to see how disconnect is exposed
cat -n electron/preload.ts | head -80Repository: stepandel/pinecone-explorer
Length of output: 3483
🏁 Script executed:
# Check pinecone-service disconnect implementation
sed -n '1570,1590p' electron/pinecone-service.tsRepository: stepandel/pinecone-explorer
Length of output: 577
Add disconnect method to ElectronAPI type definition.
The disconnect method is already implemented in the preload script and main process, but the TypeScript declaration in src/types/electron.d.ts (ElectronAPI.pinecone interface) is missing it. Add the line:
disconnect: (profileId: string) => Promise<void>to the pinecone object in the ElectronAPI interface to match the runtime implementation.
🤖 Prompt for AI Agents
In `@e2e/connection-flow.spec.ts` around lines 175 - 216, Add the missing
disconnect declaration to the ElectronAPI type so the TS interface matches the
runtime: open the ElectronAPI.pinecone interface and add a method signature for
disconnect that accepts a profileId: string and returns Promise<void> (i.e.,
declare disconnect: (profileId: string) => Promise<void> on the pinecone object)
to ensure the preload/main implemented method is typed.
Comment on lines
+155
to
+198
| test('should create new index with provider-specific settings', async () => { | ||
| const { page } = electronContext | ||
|
|
||
| const hasRealApiKey = !!process.env.PINECONE_API_KEY && | ||
| process.env.PINECONE_API_KEY !== 'dummy-key-for-local-testing' | ||
|
|
||
| if (!hasRealApiKey) { | ||
| test.skip() | ||
| return | ||
| } | ||
|
|
||
| // Generate unique index name | ||
| testIndexName = `test-index-${Date.now()}` | ||
|
|
||
| // Create index with specific settings | ||
| const createParams = { | ||
| name: testIndexName, | ||
| dimension: 384, // Standard embedding dimension | ||
| metric: 'cosine' as const, | ||
| textField: '_text', | ||
| serverlessSpec: { | ||
| cloud: 'aws' as const, | ||
| region: 'us-east-1', | ||
| }, | ||
| } | ||
|
|
||
| await page.evaluate(async ({ id, params }) => { | ||
| await (window as any).electronAPI.pinecone.createIndex(id, params) | ||
| }, { id: testProfileId, params: createParams }) | ||
|
|
||
| // Wait for index to be created and become ready | ||
| // Pinecone indexes can take a while to initialize | ||
| await page.waitForTimeout(5000) | ||
|
|
||
| // Verify index was created by listing indexes | ||
| const indexes = await page.evaluate(async (id) => { | ||
| return await (window as any).electronAPI.pinecone.listIndexes(id) | ||
| }, testProfileId) | ||
|
|
||
| const createdIndex = indexes.find((idx: any) => idx.name === testIndexName) | ||
| expect(createdIndex).toBeDefined() | ||
| expect(createdIndex?.dimension).toBe(384) | ||
| expect(createdIndex?.metric).toBe('cosine') | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find CreateIndexParams definition and createIndex implementation
rg -n "CreateIndexParams|interface CreateIndexParams|type CreateIndexParams" --type tsRepository: stepandel/pinecone-explorer
Length of output: 1132
🏁 Script executed:
#!/bin/bash
# Search for createIndex IPC handler and electron pinecone service
fd "pinecone-service" -S
ls -la electron/Repository: stepandel/pinecone-explorer
Length of output: 1324
🏁 Script executed:
#!/bin/bash
# Look at the actual pinecone service implementation
rg -n "createIndex" --type ts -C 5 | head -100Repository: stepandel/pinecone-explorer
Length of output: 7129
🏁 Script executed:
#!/bin/bash
# Find the Pinecone client/API usage to understand expected parameter shape
rg -n "serverlessSpec|serverless" --type ts -C 3 | head -80Repository: stepandel/pinecone-explorer
Length of output: 4365
Fix createIndex params: use spec.serverless instead of serverlessSpec.
CreateIndexParams expects spec: { serverless: { cloud, region } }, but the test passes serverlessSpec directly. The pinecone-service.ts accesses params.spec at line 1096; passing serverlessSpec will be ignored and cause index creation to fail.
Correct parameter shape
const createParams = {
name: testIndexName,
dimension: 384, // Standard embedding dimension
metric: 'cosine' as const,
textField: '_text',
- serverlessSpec: {
- cloud: 'aws' as const,
- region: 'us-east-1',
- },
+ spec: {
+ serverless: {
+ cloud: 'aws' as const,
+ region: 'us-east-1',
+ },
+ },
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| test('should create new index with provider-specific settings', async () => { | |
| const { page } = electronContext | |
| const hasRealApiKey = !!process.env.PINECONE_API_KEY && | |
| process.env.PINECONE_API_KEY !== 'dummy-key-for-local-testing' | |
| if (!hasRealApiKey) { | |
| test.skip() | |
| return | |
| } | |
| // Generate unique index name | |
| testIndexName = `test-index-${Date.now()}` | |
| // Create index with specific settings | |
| const createParams = { | |
| name: testIndexName, | |
| dimension: 384, // Standard embedding dimension | |
| metric: 'cosine' as const, | |
| textField: '_text', | |
| serverlessSpec: { | |
| cloud: 'aws' as const, | |
| region: 'us-east-1', | |
| }, | |
| } | |
| await page.evaluate(async ({ id, params }) => { | |
| await (window as any).electronAPI.pinecone.createIndex(id, params) | |
| }, { id: testProfileId, params: createParams }) | |
| // Wait for index to be created and become ready | |
| // Pinecone indexes can take a while to initialize | |
| await page.waitForTimeout(5000) | |
| // Verify index was created by listing indexes | |
| const indexes = await page.evaluate(async (id) => { | |
| return await (window as any).electronAPI.pinecone.listIndexes(id) | |
| }, testProfileId) | |
| const createdIndex = indexes.find((idx: any) => idx.name === testIndexName) | |
| expect(createdIndex).toBeDefined() | |
| expect(createdIndex?.dimension).toBe(384) | |
| expect(createdIndex?.metric).toBe('cosine') | |
| }) | |
| test('should create new index with provider-specific settings', async () => { | |
| const { page } = electronContext | |
| const hasRealApiKey = !!process.env.PINECONE_API_KEY && | |
| process.env.PINECONE_API_KEY !== 'dummy-key-for-local-testing' | |
| if (!hasRealApiKey) { | |
| test.skip() | |
| return | |
| } | |
| // Generate unique index name | |
| testIndexName = `test-index-${Date.now()}` | |
| // Create index with specific settings | |
| const createParams = { | |
| name: testIndexName, | |
| dimension: 384, // Standard embedding dimension | |
| metric: 'cosine' as const, | |
| textField: '_text', | |
| spec: { | |
| serverless: { | |
| cloud: 'aws' as const, | |
| region: 'us-east-1', | |
| }, | |
| }, | |
| } | |
| await page.evaluate(async ({ id, params }) => { | |
| await (window as any).electronAPI.pinecone.createIndex(id, params) | |
| }, { id: testProfileId, params: createParams }) | |
| // Wait for index to be created and become ready | |
| // Pinecone indexes can take a while to initialize | |
| await page.waitForTimeout(5000) | |
| // Verify index was created by listing indexes | |
| const indexes = await page.evaluate(async (id) => { | |
| return await (window as any).electronAPI.pinecone.listIndexes(id) | |
| }, testProfileId) | |
| const createdIndex = indexes.find((idx: any) => idx.name === testIndexName) | |
| expect(createdIndex).toBeDefined() | |
| expect(createdIndex?.dimension).toBe(384) | |
| expect(createdIndex?.metric).toBe('cosine') | |
| }) |
🤖 Prompt for AI Agents
In `@e2e/index-collection-management.spec.ts` around lines 155 - 198, The test
passes createParams with serverlessSpec which doesn't match the expected
CreateIndexParams shape; update createParams used in the test (the object passed
to electronAPI.pinecone.createIndex) so it nests the cloud/region under
params.spec.serverless (e.g. params.spec = { serverless: { cloud, region } })
instead of serverlessSpec, ensuring the call to
window.electronAPI.pinecone.createIndex receives the correct spec structure that
pinecone-service.ts expects when reading params.spec.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
11 comprehensive E2E tests for Pinecone namespace functionality.
Tests Included
Test Features
Running
Closes PINE-30
Summary by CodeRabbit
Tests
Documentation