fix(csharp/client): DH-22925: Migrate unit tests to TUnit#8149
Open
kosak wants to merge 2 commits into
Open
Conversation
kosak
added
NoDocumentationNeeded
NoReleaseNotesNeeded
Contributor
No docs changes detected for aa939fb |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the C# client unit tests from xUnit to TUnit, updating test attributes and assertion style across the test suite. In addition, it carries downstream changes related to list-type support/refactoring (e.g., extracting ColumnBuilder, adding list adapters, and extending Arrow/list handling), consistent with the note that this branch is downstream of #8148.
Changes:
- Convert
Dh_NetClientTestsfrom xUnit to TUnit ([Fact]/[Theory]→[Test]+Assert.That(...)style assertions). - Extract / expand Arrow+list plumbing: add
ColumnBuilder.cs,ReadOnlyListAdapters.cs, and add list-aware ColumnSource / chunk / Arrow conversion support. - Multi-target the test project (
net8.0;net10.0) and update test framework package references.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| csharp/client/Dh_NetClientTests/ViewTest.cs | Convert single test to TUnit async assertion style. |
| csharp/client/Dh_NetClientTests/ValidationTest.cs | Convert theory tests to TUnit arguments + async exception assertions. |
| csharp/client/Dh_NetClientTests/UpdateByTest.cs | Convert xUnit tests/output to TUnit + Console.WriteLine. |
| csharp/client/Dh_NetClientTests/UngroupTest.cs | Convert to TUnit + async “throws nothing” assertions. |
| csharp/client/Dh_NetClientTests/TickingTest.cs | Convert to TUnit and refactor callbacks to async/channel-based signaling. |
| csharp/client/Dh_NetClientTests/TableTest.cs | Convert to TUnit + async assertion wrapper. |
| csharp/client/Dh_NetClientTests/TableHandleAttributesTest.cs | Convert to TUnit assertions for attribute checks. |
| csharp/client/Dh_NetClientTests/StringFilterTest.cs | Convert to TUnit + use AssertionException on mismatch. |
| csharp/client/Dh_NetClientTests/SortTest.cs | Convert to TUnit + async assertion wrapper. |
| csharp/client/Dh_NetClientTests/SharableDictTest.cs | Convert to TUnit + make helper test async. |
| csharp/client/Dh_NetClientTests/SelectTest.cs | Convert to TUnit + async assertions / exception checks. |
| csharp/client/Dh_NetClientTests/ScriptTest.cs | Convert to TUnit + message-contains assertion for error case. |
| csharp/client/Dh_NetClientTests/RoundTripTest.cs | Update round-trip tests; expand nested/list coverage and remove “unsupported list” expectation. |
| csharp/client/Dh_NetClientTests/ReadOnlyListAdapterTest.cs | New tests covering list adapter behaviors across IList flavors. |
| csharp/client/Dh_NetClientTests/NullSentinelPassthroughTest.cs | Convert to TUnit assertion wrappers. |
| csharp/client/Dh_NetClientTests/MergeTest.cs | Convert to TUnit + async assertion wrapper. |
| csharp/client/Dh_NetClientTests/LastByTest.cs | Convert to TUnit + async assertion wrapper. |
| csharp/client/Dh_NetClientTests/JoinTest.cs | Convert join/aj/raj tests to TUnit. |
| csharp/client/Dh_NetClientTests/InputTableTest.cs | Convert to TUnit + minor comment rename. |
| csharp/client/Dh_NetClientTests/HeadAndTailTest.cs | Convert to TUnit + async assertion wrapper. |
| csharp/client/Dh_NetClientTests/GroupTest.cs | Convert to TUnit + update nested-list unsupported assertion style. |
| csharp/client/Dh_NetClientTests/GlobalUsings.cs | Remove xUnit global using. |
| csharp/client/Dh_NetClientTests/FilterTest.cs | Convert to TUnit + async assertion wrapper. |
| csharp/client/Dh_NetClientTests/Dh_NetClientTests.csproj | Replace xUnit deps with TUnit; multi-target net8/net10. |
| csharp/client/Dh_NetClientTests/AggregatesTest.cs | Convert to TUnit + async assertion wrapper. |
| csharp/client/Dh_NetClientTests/AddDropTest.cs | Convert to TUnit + output via Console. |
| csharp/client/Dh_NetClient/util/TableMaker.cs | Add null-handling during column building (uses ColumnBuilder). |
| csharp/client/Dh_NetClient/util/TableComparer.cs | Remove unused usings. |
| csharp/client/Dh_NetClient/util/ColumnBuilder.cs | New extracted column builder, extended for list/chunk append. |
| csharp/client/Dh_NetClient/ReadOnlyListAdapters.cs | New adapter hierarchy to present Arrow read-only lists as IList/IList. |
| csharp/client/Dh_NetClient/column/ColumnSource.cs | Add IListColumnSource alias + IHasElementType interface. |
| csharp/client/Dh_NetClient/column/ArrayColumnSource.cs | Change backing storage to nullable array form (ties into null support changes). |
| csharp/client/Dh_NetClient/chunk/ChunkMaker.cs | Add list-column chunk creation support. |
| csharp/client/Dh_NetClient/chunk/Chunk.cs | Change chunk storage type + add ListChunk alias. |
| csharp/client/Dh_NetClient/arrow_util/ArrowUtil.cs | Handle null list elements when enumerating ListArray. |
| csharp/client/Dh_NetClient/arrow_util/ArrowColumnSource.cs | Add list copier + list column source support in Arrow column sources. |
| csharp/client/Dh_NetClient/arrow_util/ArrowArrayConverter.cs | Add list-column conversion path back to Arrow arrays. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
9
to
17
| public void AddColumn<T>(string name, IReadOnlyList<T> values) { | ||
| var cb = ColumnBuilder.ForType<T>(null); | ||
| foreach (var value in values) { | ||
| if (value == null) { | ||
| cb.AppendNull(); | ||
| continue; | ||
| } | ||
| cb.Append(value); | ||
| } |
Comment on lines
24
to
33
| public sealed class Chunk<T> : Chunk { | ||
| public static Chunk<T> Create(int size) { | ||
| return new Chunk<T>(new T[size]); | ||
| return new Chunk<T>(new T?[size]); | ||
| } | ||
|
|
||
| public T[] Data { get; } | ||
| public T?[] Data { get; } | ||
|
|
||
| private Chunk(T[] data) : base(data.Length) { | ||
| private Chunk(T?[] data) : base(data.Length) { | ||
| Data = data; | ||
| } |
Comment on lines
+260
to
+277
| for (var i = 0; i < count; ++i, ++srcOffset, ++destOffset) { | ||
| if (src.IsNull(i)) { | ||
| if (nullFlags != null) { | ||
| typedDest.Data[destOffset] = null; | ||
| nullFlags.Data[destOffset] = true; | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| var slicedData = typedSrc.GetSlicedValues(srcOffset); | ||
|
|
||
| // var start = typedSrc.ValueOffsets[srcOffset]; | ||
| // var end = typedSrc.ValueOffsets[srcOffset + 1]; | ||
| // var slicedData = srcValues.Slice(start, end - start); | ||
| var sn = new AdaptorSelector(); | ||
| slicedData.Accept(sn); | ||
| typedDest.Data[destOffset] = sn.Result; | ||
| } |
Comment on lines
+13
to
18
| public async Task BadSelect(params string[] selections) { | ||
| using var ctx = CommonContextForTests.Create(new ClientOptions()); | ||
| var thm = ctx.Client.Manager; | ||
| using var staticTable = thm.EmptyTable(10); | ||
| Assert.Throws<AggregateException>(() => { | ||
| using var temp = staticTable.Select(selections); | ||
| }); | ||
| await Assert.That(() => staticTable.Select(selections)).Throws<AggregateException>(); | ||
| } |
Comment on lines
+37
to
44
| public async Task BadWhere(string condition) { | ||
| using var ctx = CommonContextForTests.Create(new ClientOptions()); | ||
| var thm = ctx.Client.Manager; | ||
| using var staticTable = thm.EmptyTable(10) | ||
| .Update("X = 12", "S = `hello`"); | ||
|
|
||
| Assert.Throws<AggregateException>(() => { | ||
| using var temp = staticTable.Where(condition); | ||
| }); | ||
| await Assert.That(() => staticTable.Where(condition)).Throws<AggregateException>(); | ||
| } |
Comment on lines
+39
to
+42
| /// or, to avoid boxing, they can cast them down to their actual concrete type. We promise that | ||
| /// the IList elements contained in these ColumnSource and Chunk Types will always implement | ||
| /// all ofIList, IList<T> and, for value types, IList<Nullable<T>>. | ||
| /// |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note to reviewers: this PR is downstream of #8148 and will look simpler once that PR is merged.
This PR changes the unit test framework used from XUnit to TUnit. It will have a bunch of superficial changes to most of the unit tests.