Fix --tags not being applied to integrated indexes, refactor /index/create.go for unit tests#51
Conversation
…ignatures for index/create.go and add unit to validate arguments are applied as we expect when passed to the SDK
|
Claude finished @austin-denoble's task —— View job PR Review Progress
|
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| // Abstracts the Pinecone Go SDK for testing purposes |
There was a problem hiding this comment.
Interface abstraction to allow passing in a mock service to represent the SDK in unit tests.
| ReadParameters: &readParams, | ||
| WriteParameters: &writeParams, | ||
| }, | ||
| Tags: indexTags, |
There was a problem hiding this comment.
This is the actual "fix" for the bug.
| assert.Equal(t, pinecone.IndexTags(options.tags), *svc.lastIntegrated.Tags) | ||
| } | ||
|
|
||
| func TestCreateIndexOptions_DeriveIndexType(t *testing.T) { |
There was a problem hiding this comment.
These tests were moved here from the old index_test.go file since they're focused on index creation anyways.
| github.com/rs/zerolog v1.32.0 | ||
| github.com/spf13/cobra v1.8.0 | ||
| github.com/spf13/viper v1.18.2 | ||
| github.com/stretchr/testify v1.10.0 |
There was a problem hiding this comment.
Pulling this in mainly to use the assert stuff for now, because I really don't want to bloat tests further with a bunch of if statements. Eventually would like to use testify for integration suites, etc.
bradumbaugh
left a comment
There was a problem hiding this comment.
@austin-denoble This approach makes sense to me, and I see the tags fix in there, too. Probably good to get someone more familiar than me with Go to take a look, though.
Problem
@bradumbaugh flagged a bug around the
--tagsargument not being applied when creating new integrated indexes through the CLI. The argument itself was not being passed to the Go SDK, which was an easy fix.Because of our general lack of unit testing, and the importance of index creation and the business logic that's applied, we really needed unit tests for this command.
Solution
There were a few functional unit tests validating some of the logic around inferring index type, but ultimately we need more coverage around the core logic inside of the
pc index createcommand.--tagsnot being applied to integrated indexes./index/create.gofunction signatures and add an interface to allow for dependency injection and unit testing. None of the logic itself has changed, but I added a newrunCreateIndexWithServicefunction and aCreateIndexServiceinterface to allow injecting a mock Go SDK to allow asserting on what gets provided.Type of Change
Test Plan
Test index creation flows for serverless, integrated, and pod indexes.