[RGen] Add helper methods for the TaskCompletionSource invocations. #23235
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces two helper methods for generating TaskCompletionSource invocations and adds corresponding unit tests.
- Adds
TcsSetExceptionandTcsSetResulthelpers inBindingSyntaxFactory - Implements test data and theory-based tests for both helpers
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.cs | Added TcsSetException and TcsSetResult methods |
| tests/rgen/Microsoft.Macios.Generator.Tests/Emitters/BindingSyntaxFactoryRuntimeTests.cs | Added test data classes and theory tests for new helpers |
Comments suppressed due to low confidence (2)
tests/rgen/Microsoft.Macios.Generator.Tests/Emitters/BindingSyntaxFactoryRuntimeTests.cs:1008
- [nitpick] The class name repeats "Tests" and could be simplified. Consider renaming to
TcsSetExceptionDataorTestDataTcsSetExceptionto avoid redundancy.
class TestDataTcsSetExceptionTests : IEnumerable<object []> {
tests/rgen/Microsoft.Macios.Generator.Tests/Emitters/BindingSyntaxFactoryRuntimeTests.cs:1032
- [nitpick] Test method names include the suffix "Tests", which is redundant. You could rename the method to
TcsSetExceptionorTcsSetException_ReturnsExpectedSyntaxfor clarity.
void TcsSetExceptionTests (string tcsVariableName, ImmutableArray<ArgumentSyntax> arguments, string expectedDeclaration)
| var argumentList = ArgumentList ( | ||
| SeparatedList<ArgumentSyntax> (arguments.ToSyntaxNodeOrTokenArray ())); | ||
| return InvocationExpression ( | ||
| MemberAccessExpression ( | ||
| SyntaxKind.SimpleMemberAccessExpression, | ||
| IdentifierName (tcsVariableName), | ||
| IdentifierName ("SetException").WithTrailingTrivia (Space))) | ||
| .WithArgumentList (argumentList); |
There was a problem hiding this comment.
[nitpick] The implementations of TcsSetException and TcsSetResult are nearly identical. Consider extracting a common method that takes the method name as a parameter to reduce duplication.
| var argumentList = ArgumentList ( | |
| SeparatedList<ArgumentSyntax> (arguments.ToSyntaxNodeOrTokenArray ())); | |
| return InvocationExpression ( | |
| MemberAccessExpression ( | |
| SyntaxKind.SimpleMemberAccessExpression, | |
| IdentifierName (tcsVariableName), | |
| IdentifierName ("SetException").WithTrailingTrivia (Space))) | |
| .WithArgumentList (argumentList); | |
| return TcsSetMethod(tcsVariableName, "SetException", arguments); |
Copilot uses AI. Check for mistakes.
mandel-macaque
force-pushed
the
dev/mandel/tcs-set-methods
branch
from
51af7ed to
2d7044a
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
) This commit allows to generate the strong delegate for a weak delegate in a bindings. The API provides two ways to configure the strong property: 1. StrongDelegateType: provide a Type for the new property. 2. StrongDelegateName: Allow to choose the generated property name. If empty the name will be the name of the WeakDelefate with the Weak prefix removed. We have some tests updates: 1. Remove the warnings from the tests. Some are due to the API being experimenta, other due to a rename. We will remove all errors in a comming PR. 2. Remove the strong delegate property from the test source since it is now generated. This completes the work for support properties (so far). --------- Co-authored-by: GitHub Actions Autoformatter <[email protected]>
Create two helper methods that will allow to interact with a tcs which is used in Async methods.
mandel-macaque
force-pushed
the
dev/mandel/tcs-set-methods
branch
from
1f4998b to
b9003d5
Compare
✅ [CI Build #0cb2ab2] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #0cb2ab2] Build passed (Detect API changes) ✅Pipeline on Agent |
❗ API diff for current PR / commit (Breaking changes).NET ( ❗ Breaking changes ❗ )✅ API diff vs stable.NET ( No breaking changes )ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [CI Build #0cb2ab2] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #0cb2ab2] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #0cb2ab2] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #0cb2ab2] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #0cb2ab2] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
🔥 [CI Build #0cb2ab2] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 2 tests failed, 113 tests passed. Failures❌ monotouch tests (iOS)
Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
|
Failing tests are unrelated to code changes. |
mandel-macaque
merged commit de62f1b
into
dev/mandel/bump-nuget-deps
3 participants
Create two helper methods that will allow to interact with a tcs which is used in Async methods.