[release/9.0] Update dependencies from dotnet/arcade#27
Conversation
…127.5 On relative base path root Microsoft.SourceBuild.Intermediate.arcade , Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Installers , Microsoft.DotNet.Build.Tasks.Templating , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.RemoteExecutor From Version 9.0.0-beta.25562.4 -> To Version 9.0.0-beta.25577.5
…otnet#64326) (cherry picked from commit c4f8522)
…meUser (dotnet#59992) (cherry picked from commit 2f9d787)
…sLiteral test Port of same change from 8.0 in dotnet#63565
…211.4 On relative base path root Microsoft.SourceBuild.Intermediate.arcade , Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Installers , Microsoft.DotNet.Build.Tasks.Templating , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.RemoteExecutor From Version 9.0.0-beta.25562.4 -> To Version 9.0.0-beta.25611.4
…226.6 On relative base path root Microsoft.SourceBuild.Intermediate.arcade , Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Installers , Microsoft.DotNet.Build.Tasks.Templating , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.RemoteExecutor From Version 9.0.0-beta.25562.4 -> To Version 9.0.0-beta.25626.6
WalkthroughMultiple .NET Arcade SDK dependencies updated to version 9.0.0-beta.25626.6 with SHA refreshes. Build infrastructure updated with pool image changes, branch conditionals, and internal feed redirects. PowerShell scripts modified with basic parsing flags and Visual Studio requirement handling. Test suites updated with regex patterns, quarantine markers, and WebSocketWrapper refactoring. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/Components/test/E2ETest/ServerExecutionTests/WebSocketCompressionTests.cs`:
- Around line 107-110: The assertion in WebSocketCompressionTests.cs currently
requires the log message to match both ParseErrorMessageRegexOld and
ParseErrorMessageRegexNew (using &&) which is wrong; update the Assert.True call
in the test (the line referencing Assert.True, ParseErrorMessageRegexOld, and
ParseErrorMessageRegexNew) to use the logical OR (||) so the assertion passes if
the message matches either regex; keep the existing failure message text or
adjust it to reflect "either" pattern if you prefer.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
eng/Version.Details.xmleng/Versions.propseng/common/core-templates/job/source-build.ymleng/common/core-templates/job/source-index-stage1.ymleng/common/core-templates/steps/source-build.ymleng/common/internal-feed-operations.ps1eng/common/post-build/nuget-verification.ps1eng/common/tools.ps1global.jsonsrc/Components/test/E2ETest/ServerExecutionTests/WebSocketCompressionTests.cssrc/SignalR/server/StackExchangeRedis/test/RedisEndToEnd.cs
🧰 Additional context used
🧬 Code graph analysis (1)
src/Components/test/E2ETest/ServerExecutionTests/WebSocketCompressionTests.cs (1)
src/Shared/Http2cat/Http2Utilities.cs (2)
Assert(1003-1044)True(1005-1011)
🔇 Additional comments (15)
eng/common/post-build/nuget-verification.ps1 (1)
68-68: LGTM!Adding
-UseBasicParsingtoInvoke-WebRequestis a best practice for CI/build environments. It avoids dependency on Internet Explorer's DOM parsing engine, which may not be available in headless or server environments, improving reliability.global.json (1)
30-31: LGTM!The SDK version updates to
9.0.0-beta.25626.6are consistent for bothMicrosoft.DotNet.Arcade.SdkandMicrosoft.DotNet.Helix.Sdk, aligning with the related version updates ineng/Version.Details.xmlandeng/Versions.props.eng/common/core-templates/job/source-build.yml (1)
68-68: LGTM!Updating the pool image from
1es-mariner-2to1es-azurelinux-3reflects Microsoft's rebranding and evolution from CBL-Mariner to Azure Linux. Azure Linux 3 is the current supported version for 1ES pools.eng/common/internal-feed-operations.ps1 (1)
29-29: LGTM!Adding
-UseBasicParsingto the credential provider download is consistent with the other PowerShell script updates in this PR, ensuring reliable web requests in CI environments without IE dependencies.eng/common/core-templates/steps/source-build.yml (1)
44-44: LGTM!The internal runtime source feed URL change from blob storage to
https://ci.dot.net/internalaligns with the broader infrastructure updates in this PR. The authentication key variable remains unchanged, indicating credential compatibility.eng/common/core-templates/job/source-index-stage1.yml (1)
9-9: Behavioral change: default condition now restricts execution to main branch.The default
conditionparameter changed from an empty string (unconditional execution) toeq(variables['Build.SourceBranch'], 'refs/heads/main'). This means the source-index stage1 job will now only run on the main branch by default.Repositories consuming this template without explicitly specifying a
conditionparameter will see this job skipped on non-main branches. If this is intentional to reduce unnecessary indexing workloads, the change is reasonable. If any consumers depend on the previous unconditional default behavior, they will need to override this parameter.src/Components/test/E2ETest/ServerExecutionTests/WebSocketCompressionTests.cs (1)
113-117: No additional feedback.src/SignalR/server/StackExchangeRedis/test/RedisEndToEnd.cs (5)
92-93: Quarantine marker looks good.
188-189: Quarantine marker looks good.
213-214: Cleanup order looks fine.
362-362: Unsealing the wrapper is fine.
406-410: TCS reset placement looks good.eng/Versions.props (1)
169-172: The Arcade version 9.0.0-beta.25626.6 is consistent across all configuration files. The four entries in eng/Versions.props (lines 169-172) match their corresponding dependencies in eng/Version.Details.xml and are properly reflected in global.json's msbuild-sdks configuration.eng/common/tools.ps1 (2)
550-575: No action needed.The only call site to
LocateVisualStudio(inInitializeVisualStudioMSBuild) always explicitly initializes$vsRequirementsbefore passing it to the function. No callers depend on implicit VS requirements fromLocateVisualStudiosince the function's sole caller ensures explicit requirements are always provided.Likely an incorrect or invalid review comment.
267-270: The-UseBasicParsingflag is safe and supported across all current PowerShell versions.
-UseBasicParsingis fully accepted in both Windows PowerShell 5.1 and PowerShell 7+. While Microsoft deprecated it and made it a no-op in PowerShell 6.0+ (basic parsing is now the default), it remains supported for backward compatibility and will not cause runtime failures. Using it ensures consistent behavior across PowerShell versions, so no action is needed.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| Assert.True( | ||
| ParseErrorMessageRegexOld.IsMatch(logs[0].Message) && | ||
| ParseErrorMessageRegexNew.IsMatch(logs[0].Message), | ||
| $"Expected log message to match one of the CSP error patterns: {ParseErrorMessageRegexOld} or {ParseErrorMessageRegexNew}. Actual: {logs[0].Message}"); |
There was a problem hiding this comment.
Use OR when matching either CSP pattern.
The condition requires a log message to match both patterns, which is effectively impossible here. Use || to match either pattern as the message says.
🛠️ Proposed fix
- Assert.True(
- ParseErrorMessageRegexOld.IsMatch(logs[0].Message) &&
- ParseErrorMessageRegexNew.IsMatch(logs[0].Message),
+ Assert.True(
+ ParseErrorMessageRegexOld.IsMatch(logs[0].Message) ||
+ ParseErrorMessageRegexNew.IsMatch(logs[0].Message),
$"Expected log message to match one of the CSP error patterns: {ParseErrorMessageRegexOld} or {ParseErrorMessageRegexNew}. Actual: {logs[0].Message}");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Assert.True( | |
| ParseErrorMessageRegexOld.IsMatch(logs[0].Message) && | |
| ParseErrorMessageRegexNew.IsMatch(logs[0].Message), | |
| $"Expected log message to match one of the CSP error patterns: {ParseErrorMessageRegexOld} or {ParseErrorMessageRegexNew}. Actual: {logs[0].Message}"); | |
| Assert.True( | |
| ParseErrorMessageRegexOld.IsMatch(logs[0].Message) || | |
| ParseErrorMessageRegexNew.IsMatch(logs[0].Message), | |
| $"Expected log message to match one of the CSP error patterns: {ParseErrorMessageRegexOld} or {ParseErrorMessageRegexNew}. Actual: {logs[0].Message}"); |
🤖 Prompt for AI Agents
In
`@src/Components/test/E2ETest/ServerExecutionTests/WebSocketCompressionTests.cs`
around lines 107 - 110, The assertion in WebSocketCompressionTests.cs currently
requires the log message to match both ParseErrorMessageRegexOld and
ParseErrorMessageRegexNew (using &&) which is wrong; update the Assert.True call
in the test (the line referencing Assert.True, ParseErrorMessageRegexOld, and
ParseErrorMessageRegexNew) to use the logical OR (||) so the assertion passes if
the message matches either regex; keep the existing failure message text or
adjust it to reflect "either" pattern if you prefer.
4 participants
Benchmark PR from agentic-review-benchmarks#2
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.