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.

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

chore: make sure storage tests use unique stores #1433

Merged
merged 2 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 17 additions & 3 deletions packages/client-sdk-nodejs/test/integration/integration-setup.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import {testCacheName} from '@gomomento/common-integration-tests';
import {
createStoreIfNotExists,
deleteStoreIfExists,
testCacheName,
testStoreName,
} from '@gomomento/common-integration-tests';
import {
AuthClient,
CreateCache,
Expand Down Expand Up @@ -249,11 +254,20 @@ export function SetupStorageIntegrationTest(): {
storageClient: PreviewStorageClient;
integrationTestStoreName: string;
} {
const {integrationTestCacheName} = SetupIntegrationTest();
const integrationTestStoreName = testStoreName();
const storageClient = momentoStorageClientForTesting();

beforeAll(async () => {
await createStoreIfNotExists(storageClient, integrationTestStoreName);
});

afterAll(async () => {
await deleteStoreIfExists(storageClient, integrationTestStoreName);
});

Comment on lines +260 to +266
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a beforeAll and afterAll to create a shared store that can be used by storage data plane tests. this will prevent leaked stores and ensure minimal creation of stores where possible.

return {
storageClient,
integrationTestStoreName: integrationTestCacheName,
integrationTestStoreName,
};
}

Expand Down
16 changes: 14 additions & 2 deletions packages/client-sdk-web/test/integration/integration-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import {
testCacheName,
isLocalhostDevelopmentMode,
createCacheIfNotExists,
testStoreName,
deleteStoreIfExists,
createStoreIfNotExists,
} from '@gomomento/common-integration-tests';
import {
CreateCache,
Expand Down Expand Up @@ -237,11 +240,20 @@ export function SetupStorageIntegrationTest(): {
storageClient: IStorageClient;
integrationTestStoreName: string;
} {
const {integrationTestCacheName} = SetupIntegrationTest();
const integrationTestStoreName = testStoreName();
const storageClient = momentoStorageClientForTesting();

beforeAll(async () => {
await createStoreIfNotExists(storageClient, integrationTestStoreName);
});

afterAll(async () => {
await deleteStoreIfExists(storageClient, integrationTestStoreName);
});

return {
storageClient,
integrationTestStoreName: integrationTestCacheName,
integrationTestStoreName,
};
}

Expand Down
294 changes: 127 additions & 167 deletions packages/common-integration-tests/src/storage/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@ import {
WithStore,
} from '../common-int-test-utils';
import {v4} from 'uuid';
import {sleep} from '@gomomento/sdk-core/dist/src/internal/utils';

export function runStorageServiceTests(
storageClient: IStorageClient,
testingStoreName: string
) {
): void {
describe('#create list and delete stores', () => {
it('creates a store, lists it and makes sure it exists, and then deletes it', async () => {
const storeName = testStoreName();
Expand Down Expand Up @@ -71,169 +70,26 @@ export function runStorageServiceTests(
});
it('should return AlreadyExists response if trying to create a store that already exists', async () => {
const storeName = testStoreName();
const createResponse = await storageClient.createStore(storeName);
switch (createResponse.type) {
// this is the expected response
case CreateStoreResponse.Success: {
break;
}
case CreateStoreResponse.AlreadyExists: {
break;
}
case CreateStoreResponse.Error: {
throw new Error(
`failed to create store, expected store to be able to happen, error: ${createResponse.message()} exception: ${createResponse.toString()}`
);
}
}
const alreadyExistResponse = await storageClient.createStore(storeName);
switch (alreadyExistResponse.type) {
case CreateStoreResponse.AlreadyExists: {
break;
}
case CreateStoreResponse.Error: {
throw new Error(
`failed to create store, expected AlreadyExists response, error: ${alreadyExistResponse.message()} exception: ${alreadyExistResponse.toString()}`
);
}
case CreateStoreResponse.Success: {
throw new Error(
'store already exists, we should not be able to create it again'
);
}
}
await storageClient.deleteStore(storeName);
});
});
describe('#store get put and delete', () => {
it('put get and delete key in a store', async () => {
await WithStore(storageClient, testingStoreName, async () => {
const key = v4();

// put/get an int value
const intValue = 42;
const putIntResponse = await storageClient.putInt(
testingStoreName,
key,
intValue
);
switch (putIntResponse.type) {
case StoragePutResponse.Success: {
break;
}
case StoragePutResponse.Error: {
throw new Error(
`failed to put key: ${putIntResponse.message()} ${putIntResponse.toString()}`
);
}
}
const getIntResponse = await storageClient.get(testingStoreName, key);
expectWithMessage(() => {
expect(getIntResponse.type).toEqual(StorageGetResponse.Found);
}, `expected Found, received ${getIntResponse.toString()}`);
expect(getIntResponse.value()?.int()).toEqual(intValue);

// put/get a double value
const doubleValue = 42.42;
const putDoubleResponse = await storageClient.putDouble(
testingStoreName,
key,
doubleValue
);
switch (putDoubleResponse.type) {
case StoragePutResponse.Success: {
break;
}
case StoragePutResponse.Error: {
throw new Error(
`failed to put key: ${putDoubleResponse.message()} ${putDoubleResponse.toString()}`
);
}
}
const getDoubleResponse = await storageClient.get(
testingStoreName,
key
);
expectWithMessage(() => {
expect(getDoubleResponse.type).toEqual(StorageGetResponse.Found);
}, `expected Found, received ${getDoubleResponse.toString()}`);
expect(getDoubleResponse.value()?.double()).toEqual(doubleValue);

// put/get a string value
const stringValue = v4();
const putStringResponse = await storageClient.putString(
testingStoreName,
key,
stringValue
);
switch (putStringResponse.type) {
case StoragePutResponse.Success: {
break;
}
case StoragePutResponse.Error: {
throw new Error(
`failed to put key: ${putStringResponse.message()} ${putStringResponse.toString()}`
);
}
}
const getStringResponse = await storageClient.get(
testingStoreName,
key
);
expectWithMessage(() => {
expect(getStringResponse.type).toEqual(StorageGetResponse.Found);
}, `expected Found, received ${getStringResponse.toString()}`);
expect(getStringResponse.value()?.string()).toEqual(stringValue);

// put/get a bytes value
const bytesValue = new Uint8Array([1, 2, 3, 4]);
const putBytesResponse = await storageClient.putBytes(
testingStoreName,
key,
bytesValue
);
switch (putBytesResponse.type) {
case StoragePutResponse.Success: {
// WithStore creates a store then deletes it
await WithStore(storageClient, storeName, async () => {
anitarua marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor Author

Choose a reason for hiding this comment

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

used WithStore in this test to make it more concise, but it does the same thing as before (calls create, create, delete)

const alreadyExistResponse = await storageClient.createStore(storeName);
switch (alreadyExistResponse.type) {
case CreateStoreResponse.AlreadyExists: {
break;
}
case StoragePutResponse.Error: {
case CreateStoreResponse.Error: {
throw new Error(
`failed to put key: ${putBytesResponse.message()} ${putBytesResponse.toString()}`
`failed to create store, expected AlreadyExists response, error: ${alreadyExistResponse.message()} exception: ${alreadyExistResponse.toString()}`
);
}
}
const getBytesResponse = await storageClient.get(testingStoreName, key);
expectWithMessage(() => {
expect(getBytesResponse.type).toEqual(StorageGetResponse.Found);
}, `expected Found, received ${getBytesResponse.toString()}`);
expect(getBytesResponse.value()?.bytes()).toEqual(bytesValue);

const deleteResponse = await storageClient.delete(
testingStoreName,
key
);
switch (deleteResponse.type) {
case StorageDeleteResponse.Success: {
break;
}
case StorageDeleteResponse.Error: {
case CreateStoreResponse.Success: {
throw new Error(
`failed to delete key in store: ${deleteResponse.message()} ${deleteResponse.toString()}`
'store already exists, we should not be able to create it again'
);
}
}
});
});
it('should return an undefined value for a key that doesnt exist', async () => {
await WithStore(storageClient, testingStoreName, async () => {
const key = v4();
const getResponse = await storageClient.get(testingStoreName, key);
expectWithMessage(() => {
expect(getResponse.type).toEqual(StorageGetResponse.NotFound);
}, `expected NotFound, received ${getResponse.toString()}`);
expect(getResponse.value()).toBeUndefined();
});
});
it('should return store not found error for deleting a store that doesnt exist', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved 'should return store not found error for deleting a store that doesnt exist' to the storage control plane block of tests

const storeName = testStoreName();
const deleteResponse = await storageClient.deleteStore(storeName);
Expand All @@ -253,21 +109,125 @@ export function runStorageServiceTests(
}
}
});
it('should successfully make two of the same requests after 5s retry timeout', async () => {
await WithStore(storageClient, testingStoreName, async () => {
const key = v4();
const getResponse1 = await storageClient.get(testingStoreName, key);
expectWithMessage(() => {
expect(getResponse1.type).toEqual(StorageGetResponse.NotFound);
}, `expected NotFound, received ${getResponse1.toString()}`);
});

await sleep(5000);
// Data plane tests should use the shared integration test store
// with the BeforeAll setup and AfterAll teardown
describe('#store get put and delete', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

all the storage data plane tests should be in this describe block and should use the shared store with unique keys

it('put get and delete key in a store', async () => {
const key = v4();

const getResponse2 = await storageClient.get(testingStoreName, key);
expectWithMessage(() => {
expect(getResponse2.type).toEqual(StorageGetResponse.NotFound);
}, `expected NotFound, received ${getResponse2.toString()}`);
});
// put/get an int value
const intValue = 42;
const putIntResponse = await storageClient.putInt(
testingStoreName,
key,
intValue
);
switch (putIntResponse.type) {
case StoragePutResponse.Success: {
break;
}
case StoragePutResponse.Error: {
throw new Error(
`failed to put key: ${putIntResponse.message()} ${putIntResponse.toString()}`
);
}
}
const getIntResponse = await storageClient.get(testingStoreName, key);
expectWithMessage(() => {
expect(getIntResponse.type).toEqual(StorageGetResponse.Found);
}, `expected Found, received ${getIntResponse.toString()}`);
expect(getIntResponse.value()?.int()).toEqual(intValue);

// put/get a double value
const doubleValue = 42.42;
const putDoubleResponse = await storageClient.putDouble(
testingStoreName,
key,
doubleValue
);
switch (putDoubleResponse.type) {
case StoragePutResponse.Success: {
break;
}
case StoragePutResponse.Error: {
throw new Error(
`failed to put key: ${putDoubleResponse.message()} ${putDoubleResponse.toString()}`
);
}
}
const getDoubleResponse = await storageClient.get(testingStoreName, key);
expectWithMessage(() => {
expect(getDoubleResponse.type).toEqual(StorageGetResponse.Found);
}, `expected Found, received ${getDoubleResponse.toString()}`);
expect(getDoubleResponse.value()?.double()).toEqual(doubleValue);

// put/get a string value
const stringValue = v4();
const putStringResponse = await storageClient.putString(
testingStoreName,
key,
stringValue
);
switch (putStringResponse.type) {
case StoragePutResponse.Success: {
break;
}
case StoragePutResponse.Error: {
throw new Error(
`failed to put key: ${putStringResponse.message()} ${putStringResponse.toString()}`
);
}
}
const getStringResponse = await storageClient.get(testingStoreName, key);
expectWithMessage(() => {
expect(getStringResponse.type).toEqual(StorageGetResponse.Found);
}, `expected Found, received ${getStringResponse.toString()}`);
expect(getStringResponse.value()?.string()).toEqual(stringValue);

// put/get a bytes value
const bytesValue = new Uint8Array([1, 2, 3, 4]);
const putBytesResponse = await storageClient.putBytes(
testingStoreName,
key,
bytesValue
);
switch (putBytesResponse.type) {
case StoragePutResponse.Success: {
break;
}
case StoragePutResponse.Error: {
throw new Error(
`failed to put key: ${putBytesResponse.message()} ${putBytesResponse.toString()}`
);
}
}
const getBytesResponse = await storageClient.get(testingStoreName, key);
expectWithMessage(() => {
expect(getBytesResponse.type).toEqual(StorageGetResponse.Found);
}, `expected Found, received ${getBytesResponse.toString()}`);
expect(getBytesResponse.value()?.bytes()).toEqual(bytesValue);

const deleteResponse = await storageClient.delete(testingStoreName, key);
switch (deleteResponse.type) {
case StorageDeleteResponse.Success: {
break;
}
case StorageDeleteResponse.Error: {
throw new Error(
`failed to delete key in store: ${deleteResponse.message()} ${deleteResponse.toString()}`
);
}
}
});
it('should return an undefined value for a key that doesnt exist', async () => {
const key = v4();
const getResponse = await storageClient.get(testingStoreName, key);
expectWithMessage(() => {
expect(getResponse.type).toEqual(StorageGetResponse.NotFound);
}, `expected NotFound, received ${getResponse.toString()}`);
expect(getResponse.value()).toBeUndefined();
});
});
}
Loading