Add Pinecone Local testing support in CI #77

austin-denoble · 2024-09-18T21:29:32Z

Problem

We want to add CI that validates testing the Go SDK against an instance of pclocal hosted in a Docker container.

Solution

Update the ci.yaml GitHub workflow file to add a new services block, which includes using the ghcr.io/pinecone-io/pinecone-index:latest Docker image to create and how two indexes locally (pods and serverless).
Add new /pinecone/local_test.go which defines a new LocalIntegrationTests struct, which isolates a collection of tests specifically meant for pclocal. The tests are isolated from our unit + integration test suite with the //go:build localServer build tag. Two suites are defined for pods and serverless, and each set of tests is run against both indexes. For now the test suite generates 100 vectors, upserts them to the index, tests basic Fetch, Query, Update, DescribeIndexStats, and Delete.
Add a new Run local integration tests step to the ci.yaml workflow. This triggers the new local-test.go file which tests both instances of locally hosted indexes (PINECONE_INDEX_URL_POD, PINECONE_INDEX_URL_SERVERLESS) with go test -count=1 -v ./pinecone -run TestRunLocalIntegrationSuite -tags=localServer.

To Do

I'd like to expand the number of different tests we exercise on each index. Specifically:

Metadata filtering
Sparse vectors
Multiple namespaces
List vectors

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update
Infrastructure change (CI configs, etc)
Non-code change (docs, etc)
None of the above: (explain here)

Test Plan

Make sure the CI workflow passes for this PR.

If you'd like to run the local integration tests locally, you will need to download Docker, and use docker compose up to create the same indexes on your local machine. Once you've done that, you'll need to export environment variables for the different index addresses, along with the dimension.

Here's an example:

export PINECONE_INDEX_URL_POD="http://localhost:5082" PINECONE_INDEX_URL_SERVERLESS="http://localhost:5081" PINECONE_DIMENSION="1536"

Then, you can run the local integration tests directly:

go test -count=1 -v ./pinecone -run TestRunLocalIntegrationSuite -tags=localServer

To see the specific tasks where the Asana app for GitHub is being used, see below:
- https://app.asana.com/0/0/1208056331944375

austin-denoble · 2024-09-19T04:21:56Z

.github/workflows/ci.yaml

@@ -17,6 +36,13 @@ jobs:
        run: |
          go get ./pinecone
      - name: Run tests
+        continue-on-error: true


We still want to run the subsequent step that specifically runs local integration tests.

austin-denoble · 2024-09-19T04:23:23Z

pinecone/local_test.go

@@ -0,0 +1,154 @@
+//go:build localServer


Isolating these tests from the other tests. I think this works for not running these tests as a part of the unit / integration test suite, but there may be a better way to do this in Go.

austin-denoble · 2024-09-19T04:30:34Z

pinecone/test_suite.go

@@ -54,7 +54,6 @@ func (ts *IntegrationTests) SetupSuite() {
 	for i, v := range vectors {
 		vectorIds[i] = v.Id
 	}
-	ts.vectorIds = append(ts.vectorIds, vectorIds...)


This was being done twice. I think it's better to keep the actual appending where we upsert successfully. It's inside the upsertVectors helper function.

…docker image, add new local_test.go harness for handling specific testing scenarios

…tep fails, pass the service name and port rather than localhost as environment variables

…index and namespace combinations, make sure we're using sparse vectors and attaching metadata, cover more deletion cases when the suite cleans up

… amount we were previously checking for because of multiple namespaces

…ctorsResponse, only run test for list vectors when the test suite is for serverless, update serverless pclocal to dot-product in github ci

…back, etc

haruska

👍

This is a good setup. It'll be great if we can eventually eliminate some of the integration testing against prod db.

haruska · 2024-09-20T15:17:22Z

pinecone/index_connection.go

+	var usage *Usage
+	if res.Usage != nil {
+		usage = &Usage{ReadUnits: derefOrDefault(res.Usage.ReadUnits, 0)}
+	} else {
+		usage = nil
+	}
+
 	return &ListVectorsResponse{
 		VectorIds:           vectorIds,
-		Usage:               &Usage{ReadUnits: derefOrDefault(res.Usage.ReadUnits, 0)},
+		Usage:               usage,


austin-denoble commented Sep 19, 2024

View reviewed changes

austin-denoble requested a review from haruska September 19, 2024 07:27

austin-denoble added 10 commits September 19, 2024 03:27

update ci workflow to run local integration test steps using pclocal …

d679ec3

…docker image, add new local_test.go harness for handling specific testing scenarios

update ci workflow to run local integration tests even if the tests s…

3d56e16

…tep fails, pass the service name and port rather than localhost as environment variables

fix go test command

72a486e

run local_test.go directly, fix host environment variable names

b532193

update go test command again, fix targeting package

c43e551

add logs, tweak add -tags=localServer

9ce2de2

fix assert in TestQueryVectors

8945c6f

try targeting different ports

fc1aabb

try localhost in CI environment

2e6773e

add basic tests for fetch, Query, Update, and DescribeIndexStats

165e7f4

austin-denoble force-pushed the adenoble/pclocal-ci-support branch from 34048a3 to 165e7f4 Compare September 19, 2024 07:30

austin-denoble marked this pull request as ready for review September 19, 2024 07:33

austin-denoble added 5 commits September 19, 2024 19:51

update pinecone local test suite to test across a large dimension of …

be98dd7

…index and namespace combinations, make sure we're using sparse vectors and attaching metadata, cover more deletion cases when the suite cleans up

make sure to actually delete all the vectors and check for double the…

7e3913a

… amount we were previously checking for because of multiple namespaces

fix checking indexes after cleanup

2912ad7

only delete by filter for cleanup on pod indexes

34b1c09

fix possible access of nil on ListResponse.Usage when creating ListVe…

7a54b5c

…ctorsResponse, only run test for list vectors when the test suite is for serverless, update serverless pclocal to dot-product in github ci

austin-denoble changed the title ~~Add Pinecone Local Testing support in CI~~ Add Pinecone Local testing support in CI Sep 20, 2024

add additional query tests, filter by metadata, test we get metadata …

c1a037f

…back, etc

haruska approved these changes Sep 20, 2024

View reviewed changes

haruska reviewed Sep 20, 2024

View reviewed changes

use toUsage instead of something inline

5e3958f

austin-denoble merged commit 7c4bc23 into main Sep 20, 2024
2 checks passed

austin-denoble deleted the adenoble/pclocal-ci-support branch September 20, 2024 15:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Pinecone Local testing support in CI #77

Add Pinecone Local testing support in CI #77

austin-denoble commented Sep 18, 2024 •

edited

Loading

austin-denoble Sep 19, 2024

austin-denoble Sep 19, 2024

austin-denoble Sep 19, 2024

haruska left a comment •

edited

Loading

haruska Sep 20, 2024

Add Pinecone Local testing support in CI #77

Add Pinecone Local testing support in CI #77

Conversation

austin-denoble commented Sep 18, 2024 • edited Loading

Problem

Solution

To Do

Type of Change

Test Plan

austin-denoble Sep 19, 2024

Choose a reason for hiding this comment

austin-denoble Sep 19, 2024

Choose a reason for hiding this comment

austin-denoble Sep 19, 2024

Choose a reason for hiding this comment

haruska left a comment • edited Loading

Choose a reason for hiding this comment

haruska Sep 20, 2024

Choose a reason for hiding this comment

austin-denoble commented Sep 18, 2024 •

edited

Loading

haruska left a comment •

edited

Loading