Skip to content

Commit bb99644

Browse files
westey-mwestey-m
authored
.Net: Fix integration test and bugs (#12135)
### Motivation and Context <!-- Thank you for your contribution to the semantic-kernel repo! Please help reviewers and future users, providing the following information: 1. Why is this change required? 2. What problem does it solve? 3. What scenario does it contribute to? 4. If it fixes an open issue, please link to the issue here. --> ### Description <!-- Describe your changes, the overall approach, the underlying design. These notes will help understanding how your code works. Thanks! --> ### Contribution Checklist <!-- Before submitting this PR, please make sure: --> - [ ] The code builds clean without any errors or warnings - [ ] The PR follows the [SK Contribution Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) and the [pre-submission formatting script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts) raises no violations - [ ] All unit tests pass, and I have added new tests where possible - [ ] I didn't break anyone 😄
1 parent 357c000 commit bb99644

17 files changed

+78
-79
lines changed

‎dotnet/src/Connectors/Connectors.Memory.CosmosNoSql/CosmosNoSqlDynamicCollection.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@ public sealed class CosmosNoSqlDynamicCollection : CosmosNoSqlCollection<object,
2424
[RequiresUnreferencedCode("The Cosmos NoSQL provider is currently incompatible with trimming.")]
2525
[RequiresDynamicCode("The Cosmos NoSQL provider is currently incompatible with NativeAOT.")]
2626
public CosmosNoSqlDynamicCollection(Database database, string name, CosmosNoSqlCollectionOptions options)
27-
: base(new(database.Client, ownsClient: false), _ => database, name, options)
27+
: this(
28+
new(database.Client, ownsClient: false),
29+
_ => database,
30+
name,
31+
options)
2832
{
2933
}
3034

@@ -44,8 +48,8 @@ public CosmosNoSqlDynamicCollection(
4448
string collectionName,
4549
CosmosClientOptions? clientOptions = null,
4650
CosmosNoSqlCollectionOptions? options = null)
47-
: base(
48-
new ClientWrapper(new CosmosClient(connectionString, clientOptions), ownsClient: true),
51+
: this(
52+
new(new CosmosClient(connectionString, clientOptions), ownsClient: true),
4953
client => client.GetDatabase(databaseName),
5054
collectionName,
5155
options)

‎dotnet/src/Connectors/Connectors.Memory.SqliteVec/SqliteMapper.cs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -71,17 +71,19 @@ public TRecord MapFromStorageToDataModel(Dictionary<string, object?> storageMode
7171

7272
var memory = SqlitePropertyMapping.MapVectorForDataModel(vectorBytes);
7373

74-
property.SetValueAsObject(record, property.Type switch
75-
{
76-
var t when t == typeof(ReadOnlyMemory<float>) => memory,
77-
var t when t == typeof(Embedding<float>) => new Embedding<float>(memory),
78-
var t when t == typeof(float[])
79-
=> MemoryMarshal.TryGetArray(memory, out ArraySegment<float> segment) && segment.Count == segment.Array!.Length
80-
? segment.Array
81-
: memory.ToArray(),
82-
83-
_ => throw new UnreachableException()
84-
});
74+
property.SetValueAsObject(
75+
record,
76+
(Nullable.GetUnderlyingType(property.Type) ?? property.Type) switch
77+
{
78+
var t when t == typeof(ReadOnlyMemory<float>) => memory,
79+
var t when t == typeof(Embedding<float>) => new Embedding<float>(memory),
80+
var t when t == typeof(float[])
81+
=> MemoryMarshal.TryGetArray(memory, out ArraySegment<float> segment) && segment.Count == segment.Array!.Length
82+
? segment.Array
83+
: memory.ToArray(),
84+
85+
_ => throw new UnreachableException()
86+
});
8587
}
8688
}
8789

‎dotnet/src/Connectors/Connectors.Memory.Weaviate/WeaviateQueryBuilder.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,9 @@ private static string BuildLegacyFilter(
245245
throw new InvalidOperationException($"Property name '{propertyName}' provided as part of the filter clause is not a valid property name.");
246246
}
247247

248-
var operand = $$"""{ path: ["{{property.StorageName}}"], operator: {{filterOperator}}, {{filterValueType}}: {{propertyValue}} }""";
248+
var storageName = property is KeyPropertyModel ? WeaviateConstants.ReservedKeyPropertyName : property.StorageName;
249+
250+
var operand = $$"""{ path: ["{{storageName}}"], operator: {{filterOperator}}, {{filterValueType}}: {{propertyValue}} }""";
249251

250252
operands.Add(operand);
251253
}

‎dotnet/src/IntegrationTests/Connectors/Memory/AzureAISearch/AzureAISearchVectorStoreRecordCollectionTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ public async Task ItCanUpsertAndRetrieveUsingTheDynamicMapperAsync()
371371
{
372372
Definition = fixture.VectorStoreRecordDefinition
373373
};
374-
using var sut = new AzureAISearchCollection<object, Dictionary<string, object?>>(fixture.SearchIndexClient, fixture.TestIndexName, options);
374+
using var sut = new AzureAISearchDynamicCollection(fixture.SearchIndexClient, fixture.TestIndexName, options);
375375

376376
// Act
377377
var baseSetGetResult = await sut.GetAsync("BaseSet-1", new RecordRetrievalOptions { IncludeVectors = true });

‎dotnet/src/IntegrationTests/Connectors/Memory/CosmosMongoDB/CosmosMongoCollectionTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ public async Task ItCanUpsertAndRetrieveUsingTheDynamicMapperAsync()
421421
Definition = fixture.HotelVectorStoreRecordDefinition
422422
};
423423

424-
using var sut = new CosmosMongoCollection<object, Dictionary<string, object?>>(fixture.MongoDatabase, fixture.TestCollection, options);
424+
using var sut = new CosmosMongoDynamicCollection(fixture.MongoDatabase, fixture.TestCollection, options);
425425

426426
// Act
427427
await sut.UpsertAsync(new Dictionary<string, object?>

‎dotnet/src/IntegrationTests/Connectors/Memory/CosmosNoSql/CosmosNoSqlCollectionTests.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,7 @@ public async Task ItCanUpsertRecordAsync()
244244
// Act
245245
record.HotelName = "Updated name";
246246
record.HotelRating = 10;
247+
await sut.UpsertAsync(record);
247248

248249
getResult = await sut.GetAsync(HotelId);
249250

@@ -352,7 +353,7 @@ public async Task ItCanUpsertAndRetrieveUsingTheDynamicMapperAsync()
352353
Definition = this.GetTestHotelRecordDefinition()
353354
};
354355

355-
using var sut = new CosmosNoSqlCollection<object, Dictionary<string, object?>>(fixture.Database!, "dynamic-mapper", options);
356+
using var sut = new CosmosNoSqlDynamicCollection(fixture.Database!, "dynamic-mapper", options);
356357

357358
await sut.EnsureCollectionExistsAsync();
358359

‎dotnet/src/IntegrationTests/Connectors/Memory/CosmosNoSql/CosmosNoSqlVectorStoreTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ namespace SemanticKernel.IntegrationTests.Connectors.Memory.CosmosNoSql;
88
/// <summary>
99
/// Integration tests for <see cref="CosmosNoSqlVectorStore"/>.
1010
/// </summary>
11-
[Collection("CosmosNoSqlVectorStoreCollection")]
11+
[Collection("CosmosNoSqlCollection")]
1212
[CosmosNoSqlConnectionStringSetCondition]
1313
public sealed class CosmosNoSqlVectorStoreTests(CosmosNoSqlVectorStoreFixture fixture)
1414
#pragma warning disable CA2000 // Dispose objects before losing scope

‎dotnet/src/IntegrationTests/Connectors/Memory/MongoDB/MongoDBVectorStoreRecordCollectionTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ public async Task ItCanUpsertAndRetrieveUsingTheDynamicMapperAsync()
420420
Definition = fixture.HotelVectorStoreRecordDefinition
421421
};
422422

423-
using var sut = new MongoCollection<object, Dictionary<string, object?>>(fixture.MongoDatabase, fixture.TestCollection, options);
423+
using var sut = new MongoDynamicCollection(fixture.MongoDatabase, fixture.TestCollection, options);
424424

425425
// Act
426426
await sut.UpsertAsync(new Dictionary<string, object?>

‎dotnet/src/IntegrationTests/Connectors/Memory/PgVector/PostgresVectorStoreFixture.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,14 @@ public VectorStoreCollection<TKey, TRecord> GetCollection<TKey, TRecord>(
6262
return vectorStore.GetCollection<TKey, TRecord>(collectionName, recordDefinition);
6363
}
6464

65+
public VectorStoreCollection<object, Dictionary<string, object?>> GetDynamicCollection(
66+
string collectionName,
67+
VectorStoreRecordDefinition recordDefinition)
68+
{
69+
var vectorStore = this.VectorStore;
70+
return vectorStore.GetDynamicCollection(collectionName, recordDefinition);
71+
}
72+
6573
/// <summary>
6674
/// Create / Recreate postgres docker container and run it.
6775
/// </summary>

‎dotnet/src/IntegrationTests/Connectors/Memory/PgVector/PostgresVectorStoreRecordCollectionTests.cs

Lines changed: 11 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -149,16 +149,15 @@ public async Task ItCanGetAndDeleteRecordAsync<TKey>(Type idType, TKey? key)
149149
var collectionName = "DeleteRecord";
150150
var sut = this.GetCollection(idType, collectionName);
151151

152-
await sut.CreateCollectionAsync();
152+
await sut.EnsureCollectionExistsAsync();
153153

154154
try
155155
{
156156
var record = this.CreateRecord<TKey>(idType, key!);
157157
var recordKey = record.HotelId;
158-
var upsertResult = await sut.UpsertAsync(record);
158+
await sut.UpsertAsync(record);
159159
var getResult = await sut.GetAsync(recordKey);
160160

161-
Assert.Equal(key, upsertResult);
162161
Assert.NotNull(getResult);
163162

164163
// Act
@@ -172,7 +171,7 @@ public async Task ItCanGetAndDeleteRecordAsync<TKey>(Type idType, TKey? key)
172171
finally
173172
{
174173
// Cleanup
175-
await sut.DeleteCollectionAsync();
174+
await sut.EnsureCollectionDeletedAsync();
176175
}
177176
}
178177

@@ -282,7 +281,7 @@ public async Task ItCanUpsertAndRetrieveUsingTheDynamicMapperAsync()
282281
{
283282
const int HotelId = 5;
284283

285-
var sut = fixture.GetCollection<object, Dictionary<string, object?>>("DynamicMapperWithNumericKey", GetVectorStoreRecordDefinition<int>());
284+
var sut = fixture.GetDynamicCollection("DynamicMapperWithNumericKey", GetVectorStoreRecordDefinition<int>());
286285

287286
await sut.EnsureCollectionExistsAsync();
288287

@@ -448,10 +447,7 @@ public async Task ItCanUpsertAndGetEnumerableTypesAsync()
448447
{
449448
Id = 1,
450449
ListInts = new() { 1, 2, 3 },
451-
CollectionInts = new HashSet<int>() { 4, 5, 6 },
452-
EnumerableInts = [7, 8, 9],
453-
ReadOnlyCollectionInts = new List<int> { 10, 11, 12 },
454-
ReadOnlyListInts = new List<int> { 13, 14, 15 }
450+
ArrayInts = new[] { 4, 5, 6 },
455451
};
456452

457453
// Act
@@ -467,27 +463,11 @@ public async Task ItCanUpsertAndGetEnumerableTypesAsync()
467463
Assert.Equal(1, getResult.ListInts![0]);
468464
Assert.Equal(2, getResult.ListInts![1]);
469465
Assert.Equal(3, getResult.ListInts![2]);
470-
Assert.NotNull(getResult.CollectionInts);
471-
Assert.Equal(3, getResult.CollectionInts!.Count);
472-
Assert.Contains(4, getResult.CollectionInts);
473-
Assert.Contains(5, getResult.CollectionInts);
474-
Assert.Contains(6, getResult.CollectionInts);
475-
Assert.NotNull(getResult.EnumerableInts);
476-
Assert.Equal(3, getResult.EnumerableInts!.Count());
477-
Assert.Equal(7, getResult.EnumerableInts.ElementAt(0));
478-
Assert.Equal(8, getResult.EnumerableInts.ElementAt(1));
479-
Assert.Equal(9, getResult.EnumerableInts.ElementAt(2));
480-
Assert.NotNull(getResult.ReadOnlyCollectionInts);
481-
Assert.Equal(3, getResult.ReadOnlyCollectionInts!.Count);
482-
var readOnlyCollectionIntsList = getResult.ReadOnlyCollectionInts.ToList();
483-
Assert.Equal(10, readOnlyCollectionIntsList[0]);
484-
Assert.Equal(11, readOnlyCollectionIntsList[1]);
485-
Assert.Equal(12, readOnlyCollectionIntsList[2]);
486-
Assert.NotNull(getResult.ReadOnlyListInts);
487-
Assert.Equal(3, getResult.ReadOnlyListInts!.Count);
488-
Assert.Equal(13, getResult.ReadOnlyListInts[0]);
489-
Assert.Equal(14, getResult.ReadOnlyListInts[1]);
490-
Assert.Equal(15, getResult.ReadOnlyListInts[2]);
466+
Assert.NotNull(getResult.ArrayInts);
467+
Assert.Equal(3, getResult.ArrayInts!.Length);
468+
Assert.Equal(4, getResult.ArrayInts![0]);
469+
Assert.Equal(5, getResult.ArrayInts![1]);
470+
Assert.Equal(6, getResult.ArrayInts![2]);
491471
}
492472

493473
#region private ==================================================================================
@@ -549,16 +529,7 @@ private sealed class RecordWithEnumerables
549529
public List<int>? ListInts { get; set; }
550530

551531
[VectorStoreData]
552-
public ICollection<int>? CollectionInts { get; set; }
553-
554-
[VectorStoreData]
555-
public IEnumerable<int>? EnumerableInts { get; set; }
556-
557-
[VectorStoreData]
558-
public IReadOnlyCollection<int>? ReadOnlyCollectionInts { get; set; }
559-
560-
[VectorStoreData]
561-
public IReadOnlyList<int>? ReadOnlyListInts { get; set; }
532+
public int[]? ArrayInts { get; set; }
562533
}
563534
#pragma warning restore CA1812, CA1859
564535

‎dotnet/src/IntegrationTests/Connectors/Memory/Qdrant/QdrantVectorStoreRecordCollectionTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ public async Task ItCanUpsertAndRetrieveUsingTheDynamicMapperAsync()
406406
{
407407
Definition = fixture.HotelVectorStoreRecordDefinition
408408
};
409-
using var sut = new QdrantCollection<object, Dictionary<string, object?>>(fixture.QdrantClient, "singleVectorHotels", ownsClient: false, options);
409+
using var sut = new QdrantDynamicCollection(fixture.QdrantClient, "singleVectorHotels", ownsClient: false, options);
410410

411411
// Act
412412
var baseSetGetResult = await sut.GetAsync(11ul, new RecordRetrievalOptions { IncludeVectors = true });

‎dotnet/src/IntegrationTests/Connectors/Memory/Redis/RedisHashSetVectorStoreRecordCollectionTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ public async Task ItCanUpsertAndRetrieveUsingTheDynamicMapperAsync()
415415
PrefixCollectionNameToKeyNames = true,
416416
Definition = fixture.BasicVectorStoreRecordDefinition
417417
};
418-
using var sut = new RedisHashSetCollection<object, Dictionary<string, object?>>(fixture.Database, TestCollectionName, options);
418+
using var sut = new RedisHashSetDynamicCollection(fixture.Database, TestCollectionName, options);
419419

420420
// Act
421421
var baseSetGetResult = await sut.GetAsync("HBaseSet-1", new RecordRetrievalOptions { IncludeVectors = true });

‎dotnet/src/IntegrationTests/Connectors/Memory/Redis/RedisJsonVectorStoreRecordCollectionTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ namespace SemanticKernel.IntegrationTests.Connectors.Memory.Redis;
2424
public sealed class RedisJsonVectorStoreRecordCollectionTests(ITestOutputHelper output, RedisVectorStoreFixture fixture)
2525
{
2626
// If null, all tests will be enabled
27-
private const string SkipReason = null;
27+
private const string SkipReason = "Redis tests fail intermittently on build server";
2828

2929
private const string TestCollectionName = "jsonhotels";
3030

@@ -67,7 +67,7 @@ public async Task ItCanCreateACollectionUpsertGetAndSearchAsync(bool useRecordDe
6767
var searchResults = await sut.SearchAsync(
6868
new ReadOnlyMemory<float>(new[] { 30f, 31f, 32f, 33f }),
6969
top: 3,
70-
new() { OldFilter = new VectorSearchFilter().EqualTo("HotelCode", 10) }).ToListAsync();
70+
new() { OldFilter = new VectorSearchFilter().EqualTo("HotelCode", 10), IncludeVectors = true }).ToListAsync();
7171

7272
// Assert
7373
var collectionExistResult = await sut.CollectionExistsAsync();
@@ -434,7 +434,7 @@ public async Task ItCanUpsertAndRetrieveUsingTheDynamicMapperAsync()
434434
PrefixCollectionNameToKeyNames = true,
435435
Definition = fixture.VectorStoreRecordDefinition
436436
};
437-
using var sut = new RedisJsonCollection<object, Dictionary<string, object?>>(fixture.Database, TestCollectionName, options);
437+
using var sut = new RedisJsonDynamicCollection(fixture.Database, TestCollectionName, options);
438438

439439
// Act
440440
var baseSetGetResult = await sut.GetAsync("BaseSet-1", new RecordRetrievalOptions { IncludeVectors = true });

‎dotnet/src/IntegrationTests/Connectors/Memory/SqliteVec/SqliteVectorStoreFixture.cs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,18 @@ public SqliteCollection<TKey, TRecord> GetCollection<TKey, TRecord>(
1717
SqliteCollectionOptions? options = default)
1818
where TKey : notnull
1919
where TRecord : class
20-
{
21-
return new SqliteCollection<TKey, TRecord>(
20+
=> new(
21+
this.ConnectionString,
22+
collectionName,
23+
options);
24+
25+
public SqliteDynamicCollection GetDynamicCollection(
26+
string collectionName,
27+
SqliteCollectionOptions options)
28+
=> new(
2229
this.ConnectionString,
2330
collectionName,
2431
options);
25-
}
2632

2733
public void Dispose()
2834
{

‎dotnet/src/IntegrationTests/Connectors/Memory/SqliteVec/SqliteVectorStoreRecordCollectionTests.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ namespace SemanticKernel.IntegrationTests.Connectors.Memory.SqliteVec;
2121
[Collection("SqliteVectorStoreCollection")]
2222
public sealed class SqliteVectorStoreRecordCollectionTests(SqliteVectorStoreFixture fixture)
2323
{
24-
private const string? SkipReason = "SQLite vector search extension is required";
24+
private const string? SkipReason = null;
2525

2626
[Theory(Skip = SkipReason)]
2727
[InlineData(true)]
@@ -41,6 +41,9 @@ public async Task CollectionExistsReturnsCollectionStateAsync(bool createCollect
4141

4242
// Assert
4343
Assert.Equal(createCollection, collectionExists);
44+
45+
// Cleanup
46+
await sut.EnsureCollectionDeletedAsync();
4447
}
4548

4649
[Fact(Skip = SkipReason)]
@@ -439,7 +442,7 @@ public async Task ItCanUpsertAndRetrieveUsingTheDynamicMapperWithNumericKeyAsync
439442
Definition = GetVectorStoreRecordDefinition<long>()
440443
};
441444

442-
using var sut = fixture.GetCollection<object, Dictionary<string, object?>>("DynamicMapperWithNumericKey", options);
445+
using var sut = fixture.GetDynamicCollection("DynamicMapperWithNumericKey", options);
443446

444447
await sut.EnsureCollectionExistsAsync();
445448

@@ -479,7 +482,7 @@ public async Task ItCanUpsertAndRetrieveUsingTheDynamicMapperWithStringKeyAsync(
479482
Definition = GetVectorStoreRecordDefinition<string>()
480483
};
481484

482-
using var sut = fixture.GetCollection<object, Dictionary<string, object?>>("DynamicMapperWithStringKey", options)
485+
using var sut = fixture.GetDynamicCollection("DynamicMapperWithStringKey", options)
483486
as VectorStoreCollection<object, Dictionary<string, object?>>;
484487

485488
await sut.EnsureCollectionExistsAsync();

‎dotnet/src/IntegrationTests/Connectors/Memory/Weaviate/WeaviateVectorStoreRecordCollectionTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ public async Task ItCanUpsertAndRetrieveUsingDynamicMappingAsync()
357357
Definition = this.GetTestHotelRecordDefinition()
358358
};
359359

360-
using var sut = new WeaviateCollection<object, Dictionary<string, object?>>(fixture.HttpClient!, "TestDynamicMapper", options);
360+
using var sut = new WeaviateDynamicCollection(fixture.HttpClient!, "TestDynamicMapper", options);
361361

362362
await sut.EnsureCollectionExistsAsync();
363363

0 commit comments

Comments
 (0)