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

fix: clean up after MVI control tests #509

Merged
merged 1 commit into from
Nov 1, 2023
Merged

fix: clean up after MVI control tests #509

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 6 additions & 4 deletions tests/Integration/Momento.Sdk.Tests/TtlTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public async Task UpdateTtlAsync_KeyIsByteArrayAndExists_IsSet()
// Add an item with a minute ttl
byte[] key = Utils.NewGuidByteArray();
var ttl = TimeSpan.FromSeconds(60);
var response = await client.SetAsync(cacheName, key, Utils.NewGuidByteArray());
var response = await client.SetAsync(cacheName, key, Utils.NewGuidByteArray(), ttl);
Assert.True(response is CacheSetResponse.Success, $"Unexpected response: {response}");

// Check it is there
Expand All @@ -90,7 +90,7 @@ public async Task UpdateTtlAsync_KeyIsStringAndExists_IsSet()
// Add an item with a minute ttl
string key = Utils.NewGuidString();
var ttl = TimeSpan.FromSeconds(60);
var response = await client.SetAsync(cacheName, key, Utils.NewGuidString());
var response = await client.SetAsync(cacheName, key, Utils.NewGuidString(), ttl);
Assert.True(response is CacheSetResponse.Success, $"Unexpected response: {response}");

// Check it is there
Expand Down Expand Up @@ -123,15 +123,17 @@ public async Task ItemGetTtl_HappyPath()
var ttlResponse = await client.ItemGetTtlAsync(cacheName, key);
Assert.True(ttlResponse is CacheItemGetTtlResponse.Hit, $"Unexpected response: {ttlResponse}");
var theTtl = ((CacheItemGetTtlResponse.Hit)ttlResponse).Value;
Assert.True(theTtl.TotalMilliseconds < 60000 && theTtl.TotalMilliseconds > 55000);
Assert.True(theTtl.TotalMilliseconds < 60000,
$"TTL should be less than 60 seconds but was {theTtl.TotalMilliseconds}");

await Task.Delay(1000);

var ttlResponse2 = await client.ItemGetTtlAsync(cacheName, key);
Assert.True(ttlResponse2 is CacheItemGetTtlResponse.Hit, $"Unexpected response: {ttlResponse}");
var theTtl2 = ((CacheItemGetTtlResponse.Hit)ttlResponse2).Value;

Assert.True(theTtl2.TotalMilliseconds <= theTtl.TotalMilliseconds - 1000);
Assert.True(theTtl2.TotalMilliseconds < theTtl.TotalMilliseconds,
$"TTL should be less than the previous TTL {theTtl.TotalMilliseconds} but was {theTtl2.TotalMilliseconds}");
}

[Fact]
Expand Down
47 changes: 30 additions & 17 deletions tests/Integration/Momento.Sdk.Tests/VectorIndexControlTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,43 @@ public async Task CreateListDelete_HappyPath()
{
var indexName = $"dotnet-integration-{Utils.NewGuidString()}";
const int numDimensions = 3;

var createResponse = await vectorIndexClient.CreateIndexAsync(indexName, numDimensions);
Assert.True(createResponse is CreateVectorIndexResponse.Success, $"Unexpected response: {createResponse}");

var listResponse = await vectorIndexClient.ListIndexesAsync();
Assert.True(listResponse is ListVectorIndexesResponse.Success, $"Unexpected response: {listResponse}");
var listOk = (ListVectorIndexesResponse.Success)listResponse;
Assert.Contains(listOk.IndexNames, name => name == indexName);

var deleteResponse = await vectorIndexClient.DeleteIndexesAsync(indexName);
Assert.True(deleteResponse is DeleteVectorIndexResponse.Success, $"Unexpected response: {deleteResponse}");

try
{
var createResponse = await vectorIndexClient.CreateIndexAsync(indexName, numDimensions);
Assert.True(createResponse is CreateVectorIndexResponse.Success, $"Unexpected response: {createResponse}");

var listResponse = await vectorIndexClient.ListIndexesAsync();
Assert.True(listResponse is ListVectorIndexesResponse.Success, $"Unexpected response: {listResponse}");
var listOk = (ListVectorIndexesResponse.Success)listResponse;
Assert.Contains(listOk.IndexNames, name => name == indexName);
}
finally
{
var deleteResponse = await vectorIndexClient.DeleteIndexesAsync(indexName);
Assert.True(deleteResponse is DeleteVectorIndexResponse.Success, $"Unexpected response: {deleteResponse}");
}
}

[Fact]
public async Task CreateIndexAsync_AlreadyExistsError()
{
var indexName = $"index-{Utils.NewGuidString()}";
const int numDimensions = 3;

var createResponse = await vectorIndexClient.CreateIndexAsync(indexName, numDimensions);
Assert.True(createResponse is CreateVectorIndexResponse.Success, $"Unexpected response: {createResponse}");

var createAgainResponse = await vectorIndexClient.CreateIndexAsync(indexName, numDimensions);
Assert.True(createAgainResponse is CreateVectorIndexResponse.AlreadyExists, $"Unexpected response: {createAgainResponse}");

try
{
var createResponse = await vectorIndexClient.CreateIndexAsync(indexName, numDimensions);
Assert.True(createResponse is CreateVectorIndexResponse.Success, $"Unexpected response: {createResponse}");

var createAgainResponse = await vectorIndexClient.CreateIndexAsync(indexName, numDimensions);
Assert.True(createAgainResponse is CreateVectorIndexResponse.AlreadyExists, $"Unexpected response: {createAgainResponse}");
}
finally
{
var deleteResponse = await vectorIndexClient.DeleteIndexesAsync(indexName);
Assert.True(deleteResponse is DeleteVectorIndexResponse.Success, $"Unexpected response: {deleteResponse}");
Comment on lines +54 to +55
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if we should assert that delete is successful. This clean-up finally block is best-effort. IMO we should do

finally {
  try {
     var deleteResponse = await vectorIndexClient.DeleteIndexesAsync(indexName);
  } catch (e: Exception) {
    // log and continue;
  }

}

Otherwise the failure from this finally block can mask the actual failure from the try block above.

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed. also might be good to fast follow this with a higher-order helper function called withTestVectorIndex that encapsulates the cleanup so that we have an easy repeatable pattern for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can switch to just throwing an exception. We need to avoid a passing test containing a failing delete call.

}
}

[Fact]
Expand Down
Loading