Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build out index tags tests a bit more #319

Merged
merged 4 commits into from
Jan 7, 2025
Merged

Build out index tags tests a bit more #319

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f2b54c1
Build out index tags tests a bit more
aulorbe Dec 23, 2024
9fa9108
Make index tests a bit more robust
aulorbe Jan 7, 2025
01f8cf4
Reorganize tests
aulorbe Jan 7, 2025
621fdb9
Fix comment
aulorbe Jan 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion src/integration/control/configureIndex.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,52 @@ describe('configure index', () => {
});
});

test('Add index tag to a serverless index', async () => {
const description = await pinecone.describeIndex(serverlessIndexName);
expect(description.tags).toEqual({
project: 'pinecone-integration-tests',
});

await pinecone.configureIndex(serverlessIndexName, {
tags: { testTag: 'testValue' },
});
const description2 = await pinecone.describeIndex(serverlessIndexName);
expect(description2.tags).toEqual({
project: 'pinecone-integration-tests',
testTag: 'testValue',
});
});

test('Remove index tag from serverless index', async () => {
const description = await pinecone.describeIndex(serverlessIndexName);
expect(description.tags).toEqual({
project: 'pinecone-integration-tests',
testTag: 'testValue',
});

await pinecone.configureIndex(serverlessIndexName, {
tags: { project: '' },
});
const description2 = await pinecone.describeIndex(serverlessIndexName);
expect(description2.tags).toBeUndefined();
if (description2.tags != null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If undefined != null, does this block still run?

Separately, are tags always returned, even if they're empty? I guess that would be my expectation as a user of the SDK.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question 1: undefined does equal null.

Question2 : Yes!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok, so the conflation of null and undefined are intentional here because tags should always be {} regardless of whether they've been added to the response or not? That would make sense to me!

expect(description2.tags['project']).toBeUndefined();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting. I'm sure this has already been discussed, but I'd expect to have to explicitly pass null for this to happen and for empty strings to be the case, which is how a few other SDKs we use would do it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the tag itself (the key value) is still present, but the value can be an empty string. If it's an empty string, then it's undefined. Does that make sense?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely more on the implementation side and agnostic to the SDK, but if I understand correctly that json responses don't really encode undefined, ie. the key is just omitted altogether, providing project: '' in this case would remove the key altogether?

so i'd expect something like

input: `{ "project": null }`, output: `{}`
input: `{ "project": "" }`, output: `{ "project": "" }`

whereas this test tells me

input: `{ "project": "" }`, output: `{ }`

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, so ' ' removes the key, but the user can't pass null because of type safety in typescript (their IDE will tell them that the passed value in a tag must be a string)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Same thing happpens here when you sub undefined for null:)

Screenshot 2025-01-07 at 12 52 35 PM

expect(description2.tags['testTag']).toEqual('testValue');
}
});

test('Update a tag value in a serverless index', async () => {
const description = await pinecone.describeIndex(serverlessIndexName);
expect(description.tags).toEqual({
testTag: 'testValue',
});

await pinecone.configureIndex(serverlessIndexName, {
tags: { testTag: 'newValue' },
});
const description2 = await pinecone.describeIndex(serverlessIndexName);
if (description2.tags != null) {
expect(description2.tags['testTag']).toEqual('newValue');
}
});
});

Expand Down
Loading