Skip to content

Commit

Permalink
Centralize Go test suite (#48)
Browse files Browse the repository at this point in the history
## Problem

Our current test set up is not as useful as it could be. Some issues
are:
- The IDs, metadata, and vectors used in tests are generated in
non-deterministic ways
- The (integration) tests rely on humans having to manually create
indexes in our test project; those indexes must have titles that match
Github secret env vars; if this isn't done correctly, tests fail
- The tests do not differentiate between serverless and pod indexes,
which make things cumbersome to work with (and make confirming updated
vector values, etc. quite impossible)
- Our tests are redundant and cost inefficient -- we spin up indexes
mult times and either delete them or just let them live forever in our
test project

## Solution

Make it all better! 

The current architecture now looks like this: 

Two top-level test infra files:
1. `integration_test_suite.go`: This file defines a single test `struct`
called `IntegrationTests` that holds the fields for everything we need
wrt integration testing across all `go` files in our project
a. Importantly, this file also contains the `testify` mandatory
`SetupSuite` and `TeardownSuite` methods attached to this `struct`, so
that indexes are always torn down after testing completes
2. `run_integration_test_suites.go`: This file actually runs the test
suites defined in `integration_test_suite.go` via `testify`'s
`suite.Run` command. It runs 2 suites: 1 for pods (`podTestSuite`) and 1
for serverless (`serverlessTestSuite`)

Individual test files:
Each file still has a complementary `..._test.go` file that contains its
integration and unit tests. The main difference this PR introduces is
that each of these files no longer contains a redundant `SetupSuite`
function, etc. Instead, they simply call
`run_integration_test_suites.go`'s `RunSuites()` method, and everything
is automagically created/destroyed.

## Genesis

This refactor arose from Audrey trying to [write integration tests for
`update`](https://app.asana.com/0/1203260648987893/1207680319590579/f),
but being unable to do so, since she could not easily compare IDs,
vector values or metadata before vs after `update` operations.

## Misc.:
There is still a lot of things we can do to make our tests better and
more efficient, I'm sure. This is just one baby step on the longer
journey towards test suite-maturity.

## FAQs
> Why do we need two infra-type files (`integration_test_suite.go` and
`run_integration_test_suites.go`)?

I don't like it either, but apparently this is what is needed for
`testify` to work 😢 . You can't have the `suite.Run` call in the same
file as the `SetupSuite` and `TeardownSuite` methods.

> Does the fact that all (integration) tests now share the same struct
(`IntegrationTests`) mean that when you run the integration tests in a
specific file (e.g. `client_tests.go`), all integration tests actually
run?

Yes. This obviously isn't ideal for dev work, but you can figure out how
to run individual tests via the command line by reading up on `go test`.

> Why have different `Suites` for pods vs serverless indexes, when they
share most fields?

This is totally fair and tbh I simply didn't refactor this part because
this PR is getting gigantic and it seemed like it would add unnecessary
complexity to it. But we should go over the pros/cons of having
everything in a single `Suite` later! For now, splitting them out
produced the invaluable outcome of allowing me to test different things
per index type.

 
## Type of Change

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

## Test Plan

CI passes.
  • Loading branch information
aulorbe authored Jul 26, 2024
1 parent be9fe92 commit 3da0e9d
Show file tree
Hide file tree
Showing 8 changed files with 485 additions and 318 deletions.
2 changes: 0 additions & 2 deletions .env.example
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
PINECONE_API_KEY="<Project API Key>"
TEST_PODS_INDEX_NAME="<Pod based Index name>"
TEST_SERVERLESS_INDEX_NAME="<Serverless based Index name>"
4 changes: 1 addition & 3 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ jobs:
run: |
go get ./pinecone
- name: Run tests
run: go test ./pinecone
run: go test -count=1 -v ./pinecone
env:
TEST_PODS_INDEX_NAME: ${{ secrets.TEST_PODS_INDEX_NAME }}
TEST_SERVERLESS_INDEX_NAME: ${{ secrets.TEST_SERVERLESS_INDEX_NAME }}
PINECONE_API_KEY: ${{ secrets.API_KEY }}
13 changes: 9 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@
Official Pinecone Go Client

## Documentation

To see the latest documentation on `main`, visit https://pkg.go.dev/github.com/pinecone-io/go-pinecone@main/pinecone.

To see the latest versioned-release's documentation, visit https://pkg.go.dev/github.com/pinecone-io/go-pinecone/pinecone.
To see the latest versioned-release's documentation,
visit https://pkg.go.dev/github.com/pinecone-io/go-pinecone/pinecone.

## Features

Expand Down Expand Up @@ -100,9 +102,11 @@ Then, execute `just bootstrap` to install the necessary Go packages

### .env Setup

To avoid race conditions or having to wait for index creation, the tests require a project with at least one pod index
and one serverless index. Copy the api key and index names to a `.env` file. See `.env.example` for a template.
An easy way to keep track of necessary environment variables is to create a `.env` file in the root of the project.
This project comes with a sample `.env` file (`.env.sample`) that you can copy and modify. At the very least, you
will need to include the `PINECONE_API_KEY` variable in your `.env` file for the tests to run locally.

```shell
### API Definitions submodule

The API Definitions are in a private submodule. To checkout or update the submodules execute in the root of the project:
Expand All @@ -111,7 +115,8 @@ The API Definitions are in a private submodule. To checkout or update the submod
git submodule update --init --recursive
```

For working with submodules, see the [Git Submodules](https://git-scm.com/book/en/v2/Git-Tools-Submodules) documentation.
For working with submodules, see the [Git Submodules](https://git-scm.com/book/en/v2/Git-Tools-Submodules)
documentation.

### Just commands

Expand Down
6 changes: 0 additions & 6 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@ test:
source .env
set +o allexport
go test -count=1 -v ./pinecone
test-integration:
#!/usr/bin/env bash
set -o allexport
source .env
set +o allexport
go test -v -run Integration ./pinecone
test-unit:
#!/usr/bin/env bash
set -o allexport
Expand Down
Loading

0 comments on commit 3da0e9d

Please sign in to comment.