-
Notifications
You must be signed in to change notification settings - Fork 874
[V4] Update DynamoDB tests to use async operations #4203
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
base: development
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class is not marked with the TestClass attribute so the test never runs... I deleted it since it wasn't doing anything special, we have a separate project for CSM tests: https://github.com/aws/aws-sdk-net/tree/main/sdk/test/CSMTest
| { | ||
|
|
||
| // Only run these tests if we are not reusing tables | ||
| if (ReuseTables) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ReuseTables is hard-coded to true so this test is always skipped. Also deleted since several other tests call Create / DeleteTable themselves.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request updates DynamoDB .NET Framework integration tests to use async operations consistently throughout the codebase. The changes follow up on PR #4178 and modernize the test suite by:
- Converting synchronous DynamoDB operations to their async counterparts
- Updating test method signatures from
void/Tasktoasync Task - Replacing synchronous exception assertions with async-compatible patterns
- Modernizing code patterns (e.g.,
Thread.Sleep→Task.Delay, yield return → collection building)
Key Changes
- Async operation adoption: All DynamoDB CRUD operations (Save, Load, Query, Scan, etc.) now use async methods with proper await
- Test infrastructure updates: ClassInitialize, ClassCleanup, and TestCleanup methods updated to async
- Exception handling: Migrated from
AssertExtensions.ExpectExceptiontoAssert.ThrowsExceptionAsync
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| TTLTests.cs | Converted TTL-related test methods to async, updated Context operations and document table operations |
| ServiceTests.cs | Converted service layer tests to async, refactored ScanHelper from yield return to collection-based approach |
| JSONTests.cs | Updated JSON document tests with async operations and inline document initialization |
| DynamoDBTestsBase.cs | Converted base test infrastructure (setup/teardown) to async, added parallel cleanup with Task.WhenAll |
| DocumentTests.cs | Comprehensive async conversion of document operations, replaced Thread.Sleep with Task.Delay |
| DataModelTests.cs | Converted DataModel context operations to async, updated query/scan operations |
| Cache.cs | Updated cache tests to async, added NETFRAMEWORK conditional compilation |
| CSMTest.cs | Removed deprecated test file |
| AWSSDK.IntegrationTests.DynamoDBv2.NetFramework.csproj | Added CS0618 to NoWarn to suppress obsolete API warnings |
Another follow-up for #4178, this updates the DDB .NET Framework integration tests to use async operations when possible. I tried to split one file in each commit to make the review more manageable.
Testing
DRY_RUN-82596693-e13f-425a-9515-31a47d1cc7f6License