Conversation
There was a problem hiding this comment.
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.
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); |
|
|
||
| Assert.Equal(0, packProcess.ExitCode); | ||
|
|
||
| var packageDir = Path.Combine(Path.GetDirectoryName(projectPath)!, "bin", "Release"); |
There was a problem hiding this comment.
[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); |
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.