-
Notifications
You must be signed in to change notification settings - Fork 490
Sync Dev and Master #2116
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
Sync Dev and Master #2116
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.
Pull Request Overview
This PR syncs the dev branch with master, including .NET runtime version updates, Lambda test tool improvements, and packaging modifications.
- Updates ASP.NET Core runtime versions for .NET 9 and .NET 10 Docker images
- Adds .NET 10 support to the Lambda test tool and improves directory filtering
- Removes static asset packaging configuration and adds new static asset verification tests
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/Utilities/DirectoryHelpers.cs | Excludes .vs directory from copying operations |
Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/PackagingTests.cs | Adds static asset verification test and includes .NET 10 support |
Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Amazon.Lambda.TestTool.csproj | Removes static asset packaging configuration |
LambdaRuntimeDockerfiles/Images/net9/arm64/Dockerfile | Updates ASP.NET Core from 9.0.6 to 9.0.7 |
LambdaRuntimeDockerfiles/Images/net9/amd64/Dockerfile | Updates ASP.NET Core from 9.0.6 to 9.0.7 |
LambdaRuntimeDockerfiles/Images/net10/arm64/Dockerfile | Updates ASP.NET Core from preview.5 to preview.6 |
LambdaRuntimeDockerfiles/Images/net10/amd64/Dockerfile | Updates ASP.NET Core from preview.5 to preview.6 |
Comments suppressed due to low confidence (1)
Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/PackagingTests.cs:31
- The test uses --no-build flag but there's no verification that the project was built beforehand. This could cause the test to fail if the project hasn't been built in Release configuration.
Arguments = $"pack -c Release --no-build --no-restore {projectPath}",
packProcess.Start(); | ||
string packOutput = packProcess.StandardOutput.ReadToEnd(); | ||
string packError = packProcess.StandardError.ReadToEnd(); | ||
packProcess.WaitForExit(int.MaxValue); |
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.
Using int.MaxValue as timeout could cause the test to hang indefinitely if the pack process fails. Consider using a reasonable timeout value like 30000ms (30 seconds).
packProcess.WaitForExit(int.MaxValue); | |
packProcess.WaitForExit(30000); |
Copilot uses AI. Check for mistakes.
|
||
Assert.Equal(0, packProcess.ExitCode); | ||
|
||
var packageDir = Path.Combine(Path.GetDirectoryName(projectPath)!, "bin", "Release"); |
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.
[nitpick] The hardcoded 'Release' configuration string should be consistent with the configuration used in the pack command. Consider extracting this as a constant or variable.
var packageDir = Path.Combine(Path.GetDirectoryName(projectPath)!, "bin", "Release"); | |
var packageDir = Path.Combine(Path.GetDirectoryName(projectPath)!, "bin", Configuration); |
Copilot uses AI. Check for mistakes.
Description of changes:
Sync Dev and Master
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.