feat(PINE-31): E2E-005 vector browsing tests - #29
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 reviewI found one issue that needs to be addressed: Logic Error in Qdrant Adapter: Validation-Implementation Mismatch for Sparse Vector QueriesLocation: The pinecone-explorer/electron/adapters/qdrant-adapter.ts Lines 285 to 287 in 2d3a3d2 However, the implementation only handles two cases:
pinecone-explorer/electron/adapters/qdrant-adapter.ts Lines 292 to 317 in 2d3a3d2 Issue: If a caller provides only
Suggested Fix: Either add support for sparse vector queries or remove if (params.id) {
// Query by ID
// ...
} else if (params.sparseVector) {
// Query by sparse vector
// Implement using Qdrant's sparse vector API
} else {
// Query by dense vector
const response = await this.withRetry(() =>
this.client!.search(params.collectionName, {
vector: params.vector!,
// ...
})
)
}If sparse vectors are not supported yet, update the validation: if (!params.vector && !params.id) {
throw new Error('Query requires vector or id')
} |
📝 WalkthroughWalkthroughThis pull request adds comprehensive end-to-end test suites for the Electron app, covering connection flows, index/collection management, namespace operations, and vector browsing. Four new test files totaling 2,274 lines introduce extensive test coverage primarily for Pinecone with scaffolding for Qdrant and Weaviate providers, using Playwright and IPC-based interactions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~80 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: 4
🤖 Fix all issues with AI agents
In `@e2e/connection-flow.spec.ts`:
- Around line 70-80: Replace the test API key string that triggered the secret
scanner in the profile object to a clearly non-secret placeholder (e.g.,
"INVALID_API_KEY_PLACEHOLDER") or add a gitleaks allowlist comment next to the
usage; update the code that constructs the test profile (the profile variable
and its apiKey field, and any related profileId/test-invalid string) so the
value cannot match common API-key regexes while preserving test semantics for
the connection failure scenario.
In `@e2e/namespace-operations.spec.ts`:
- Around line 155-198: Add teardown to remove test-created namespaces/vectors
after the spec: after creating testNamespace and testVectors (the code that
calls (window as any).electronAPI.pinecone.createVector and checks
getIndexStats), add an afterAll (or run cleanup immediately after assertions)
that calls the Pinecone deletion API to remove the data; use the same bridge
methods used to create/read data (e.g., (window as
any).electronAPI.pinecone.deleteVector or deleteVectors or deleteNamespace) with
testProfileId/testIndexName and the testNamespace to delete either each
testVectors[].id or the whole namespace, and apply the same cleanup for the
other tests referenced at 283-321 so repeated runs do not accumulate data.
In `@e2e/vector-browsing.spec.ts`:
- Around line 55-112: The tests currently reuse an existing namespace found via
listIndexes/getIndexStats which leads to flaky metadata types; change the logic
in this block to always create and use a dedicated deterministic namespace (set
testNamespace to a unique name like `test-vectors-${Date.now()}-${uuid}`),
always populate it with a known set of vectors (the testVectors creation and
createVector calls), skip the branch that accepts an existing namespace (remove
the early selection from stats.namespaces), and ensure teardown removes the
vectors/namespace after tests complete; update references to testNamespace and
testIndexName so all assertions use only this created namespace and keep
createVector/getIndexStats/listIndexes usage intact.
- Around line 483-486: The forEach callbacks are implicitly returning the result
of Set.add because they use expression bodies; change both forEach callbacks to
use block bodies with explicit statements (e.g., replace the arrow expressions
in vectors.forEach(...) and the nested Object.keys(v.metadata).forEach(...) so
they use { ... } and call metadataKeys.add(key); without returning anything) and
apply the same change to the other occurrence that also iterates and adds to
metadataKeys (the second vectors.forEach/... Object.keys(...).forEach instance).
🧹 Nitpick comments (1)
e2e/index-collection-management.spec.ts (1)
23-27: Make the stateful Pinecone tests serial and replace fixed sleeps with polling.These tests share
testIndexNameand depend on remote index readiness; fixedwaitForTimeoutcalls are prone to flakiness. Prefertest.describe.serialandexpect.pollon list/index stats to wait deterministically.♻️ Example adjustment for serial mode + polling
-test.describe('Pinecone Index Management', () => { +test.describe.serial('Pinecone Index Management', () => { // ... - await page.waitForTimeout(5000) + await expect.poll(async () => { + const indexes = await page.evaluate(async (id) => + (window as any).electronAPI.pinecone.listIndexes(id), + testProfileId + ) + return indexes.some((idx: any) => idx.name === testIndexName) + }, { timeout: 20000 }).toBeTruthy()- await page.waitForTimeout(3000) + await expect.poll(async () => { + const indexes = await page.evaluate(async (id) => + (window as any).electronAPI.pinecone.listIndexes(id), + testProfileId + ) + return indexes.every((idx: any) => idx.name !== testIndexName) + }, { timeout: 20000 }).toBeTruthy()Also applies to: 185-188, 217-226
Comment on lines
+70
to
+80
| // Create a profile with invalid API key and test connection via IPC | ||
| const profileId = `test-invalid-${Date.now()}` | ||
|
|
||
| const connectionFailed = await page.evaluate(async (id) => { | ||
| const profile = { | ||
| id, | ||
| name: 'Invalid Key Test', | ||
| provider: 'pinecone' as const, | ||
| apiKey: 'invalid-api-key-12345', | ||
| } | ||
|
|
There was a problem hiding this comment.
Avoid secret-scan false positives for the dummy API key.
Line 78 contains a string that matches generic API key patterns and was flagged by Gitleaks; this can fail CI despite being a test value. Use a clearly non-secret placeholder and/or an allowlist comment to prevent false positives.
🛡️ Suggested tweak to avoid secret-scanner hits
- apiKey: 'invalid-api-key-12345',
+ apiKey: 'invalid-api-key-for-tests', // gitleaks:allow📝 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
| // Create a profile with invalid API key and test connection via IPC | |
| const profileId = `test-invalid-${Date.now()}` | |
| const connectionFailed = await page.evaluate(async (id) => { | |
| const profile = { | |
| id, | |
| name: 'Invalid Key Test', | |
| provider: 'pinecone' as const, | |
| apiKey: 'invalid-api-key-12345', | |
| } | |
| // Create a profile with invalid API key and test connection via IPC | |
| const profileId = `test-invalid-${Date.now()}` | |
| const connectionFailed = await page.evaluate(async (id) => { | |
| const profile = { | |
| id, | |
| name: 'Invalid Key Test', | |
| provider: 'pinecone' as const, | |
| apiKey: 'invalid-api-key-for-tests', // gitleaks:allow | |
| } |
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 78-78: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In `@e2e/connection-flow.spec.ts` around lines 70 - 80, Replace the test API key
string that triggered the secret scanner in the profile object to a clearly
non-secret placeholder (e.g., "INVALID_API_KEY_PLACEHOLDER") or add a gitleaks
allowlist comment next to the usage; update the code that constructs the test
profile (the profile variable and its apiKey field, and any related
profileId/test-invalid string) so the value cannot match common API-key regexes
while preserving test semantics for the connection failure scenario.
Comment on lines
+155
to
+198
| // Create test namespace with a few vectors | ||
| const testNamespace = `test-ns-${Date.now()}` | ||
| const testVectors = [ | ||
| { | ||
| id: `test-vec-1`, | ||
| values: Array(dimension).fill(0).map(() => Math.random()), | ||
| metadata: { test: true, index: 1 }, | ||
| }, | ||
| { | ||
| id: `test-vec-2`, | ||
| values: Array(dimension).fill(0).map(() => Math.random()), | ||
| metadata: { test: true, index: 2 }, | ||
| }, | ||
| ] | ||
|
|
||
| // Upsert vectors to create namespace | ||
| await page.evaluate(async ({ id, indexName, namespace, vectors }) => { | ||
| for (const vector of vectors) { | ||
| await (window as any).electronAPI.pinecone.createVector(id, { | ||
| indexName, | ||
| namespace, | ||
| id: vector.id, | ||
| values: vector.values, | ||
| metadata: vector.metadata, | ||
| }) | ||
| } | ||
| }, { | ||
| id: testProfileId, | ||
| indexName: testIndexName, | ||
| namespace: testNamespace, | ||
| vectors: testVectors, | ||
| }) | ||
|
|
||
| // Wait for vectors to be indexed | ||
| await page.waitForTimeout(3000) | ||
|
|
||
| // Verify namespace was created by checking stats | ||
| const stats = await page.evaluate(async ({ id, name }) => { | ||
| return await (window as any).electronAPI.pinecone.getIndexStats(id, name) | ||
| }, { id: testProfileId, name: testIndexName }) | ||
|
|
||
| expect(stats.namespaces[testNamespace]).toBeDefined() | ||
| expect(stats.namespaces[testNamespace].vectorCount).toBe(testVectors.length) | ||
| }) |
There was a problem hiding this comment.
Clean up test-created namespaces/vectors to avoid data buildup.
The suite creates namespaces and clones them but never deletes test vectors; repeated runs can pollute real indexes and incur cost. Please add cleanup (e.g., delete all vectors in test namespaces) in afterAll or immediately after validation.
Also applies to: 283-321
🤖 Prompt for AI Agents
In `@e2e/namespace-operations.spec.ts` around lines 155 - 198, Add teardown to
remove test-created namespaces/vectors after the spec: after creating
testNamespace and testVectors (the code that calls (window as
any).electronAPI.pinecone.createVector and checks getIndexStats), add an
afterAll (or run cleanup immediately after assertions) that calls the Pinecone
deletion API to remove the data; use the same bridge methods used to create/read
data (e.g., (window as any).electronAPI.pinecone.deleteVector or deleteVectors
or deleteNamespace) with testProfileId/testIndexName and the testNamespace to
delete either each testVectors[].id or the whole namespace, and apply the same
cleanup for the other tests referenced at 283-321 so repeated runs do not
accumulate data.
Comment on lines
+55
to
+112
| // Get the first available index to use for testing | ||
| const indexes = await page.evaluate(async (id) => { | ||
| return await (window as any).electronAPI.pinecone.listIndexes(id) | ||
| }, testProfileId) | ||
|
|
||
| if (indexes.length > 0) { | ||
| testIndexName = indexes[0].name | ||
|
|
||
| // Get a namespace with vectors for testing | ||
| const stats = await page.evaluate(async ({ id, name }) => { | ||
| return await (window as any).electronAPI.pinecone.getIndexStats(id, name) | ||
| }, { id: testProfileId, name: testIndexName }) | ||
|
|
||
| // Find first namespace with vectors | ||
| for (const [name, data] of Object.entries(stats.namespaces as Record<string, any>)) { | ||
| if (data.vectorCount > 0) { | ||
| testNamespace = name | ||
| break | ||
| } | ||
| } | ||
|
|
||
| // If no namespace has vectors, create test vectors | ||
| if (!testNamespace) { | ||
| testNamespace = `test-vectors-${Date.now()}` | ||
| const dimension = indexes[0].dimension || 384 | ||
|
|
||
| // Create a few test vectors | ||
| const testVectors = Array.from({ length: 5 }, (_, i) => ({ | ||
| id: `test-vector-${i}`, | ||
| values: Array(dimension).fill(0).map(() => Math.random()), | ||
| metadata: { | ||
| test: true, | ||
| index: i, | ||
| description: `Test vector ${i}`, | ||
| category: i % 2 === 0 ? 'even' : 'odd', | ||
| }, | ||
| })) | ||
|
|
||
| for (const vector of testVectors) { | ||
| await page.evaluate(async ({ id, indexName, namespace, vector }) => { | ||
| await (window as any).electronAPI.pinecone.createVector(id, { | ||
| indexName, | ||
| namespace, | ||
| id: vector.id, | ||
| values: vector.values, | ||
| metadata: vector.metadata, | ||
| }) | ||
| }, { | ||
| id: testProfileId, | ||
| indexName: testIndexName, | ||
| namespace: testNamespace, | ||
| vector, | ||
| }) | ||
| } | ||
|
|
||
| // Wait for vectors to be indexed | ||
| await page.waitForTimeout(3000) | ||
| } |
There was a problem hiding this comment.
Make vector-browsing tests deterministic by always using a dedicated test namespace.
Right now, the suite reuses the first existing namespace with vectors, which can include arbitrary metadata schemas and types. This makes assertions (e.g., metadata type checks) flaky across environments. Consider always creating a unique test namespace with known vectors and using that for all assertions (and clean it up afterward).
🤖 Prompt for AI Agents
In `@e2e/vector-browsing.spec.ts` around lines 55 - 112, The tests currently reuse
an existing namespace found via listIndexes/getIndexStats which leads to flaky
metadata types; change the logic in this block to always create and use a
dedicated deterministic namespace (set testNamespace to a unique name like
`test-vectors-${Date.now()}-${uuid}`), always populate it with a known set of
vectors (the testVectors creation and createVector calls), skip the branch that
accepts an existing namespace (remove the early selection from
stats.namespaces), and ensure teardown removes the vectors/namespace after tests
complete; update references to testNamespace and testIndexName so all assertions
use only this created namespace and keep createVector/getIndexStats/listIndexes
usage intact.
Comment on lines
+483
to
+486
| vectors.forEach((v: any) => { | ||
| if (v.metadata) { | ||
| Object.keys(v.metadata).forEach(key => metadataKeys.add(key)) | ||
| } |
There was a problem hiding this comment.
Fix Biome lint errors: forEach callbacks must not return values.
Both callbacks currently return the Set.add result implicitly; use block bodies to avoid returning a value.
🛠️ Suggested fix for the forEach callbacks
- Object.keys(v.metadata).forEach(key => metadataKeys.add(key))
+ Object.keys(v.metadata).forEach(key => {
+ metadataKeys.add(key)
+ })- keys.forEach(k => allKeys.add(k))
+ keys.forEach(k => {
+ allKeys.add(k)
+ })Also applies to: 601-605
🧰 Tools
🪛 Biome (2.3.13)
[error] 485-485: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
In `@e2e/vector-browsing.spec.ts` around lines 483 - 486, The forEach callbacks
are implicitly returning the result of Set.add because they use expression
bodies; change both forEach callbacks to use block bodies with explicit
statements (e.g., replace the arrow expressions in vectors.forEach(...) and the
nested Object.keys(v.metadata).forEach(...) so they use { ... } and call
metadataKeys.add(key); without returning anything) and apply the same change to
the other occurrence that also iterates and adds to metadataKeys (the second
vectors.forEach/... Object.keys(...).forEach instance).
stepandel
added a commit
that referenced
this pull request
Feb 4, 2026
- Fix index creation timeout: use polling (90s) instead of fixed 5s wait - Add gitleaks:allow comments to prevent false positives on test API keys - Index creation now properly waits for status.ready=true Addresses review comments from PRs #27, #29 Co-authored-by: Stepan Arsentjev <stepandel@users.noreply.github.com>
stepandel
added a commit
that referenced
this pull request
Feb 4, 2026
* fix: address E2E test review issues - Fix index creation timeout: use polling (90s) instead of fixed 5s wait - Add gitleaks:allow comments to prevent false positives on test API keys - Index creation now properly waits for status.ready=true Addresses review comments from PRs #27, #29 * fix: address remaining E2E review issues - Add teardown in namespace-operations.spec.ts to delete test-created namespaces/vectors - Change vector-browsing.spec.ts to create dedicated test namespace with known metadata instead of reusing existing namespace - Fix forEach callbacks in vector-browsing.spec.ts to use explicit block bodies instead of implicit returns - Use test.describe.serial for stateful index create/delete tests in index-collection-management.spec.ts - Replace fixed sleeps with polling in index-collection-management.spec.ts Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> --------- Co-authored-by: Stepan Arsentjev <stepandel@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
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.
Closes PINE-31
Summary
Adds comprehensive E2E tests for vector browsing functionality (E2E-005).
Tests Added
Test Results
Summary by CodeRabbit