Skip to content

[release/9.0] Update dependencies from dotnet/arcade#12

Open
hila-f-qodo wants to merge 7 commits intodevin_pr_code_review_bench_100_devin2_base_release_90_update_dependencies_from_dotnet_arcade_pr80from
devin_pr_code_review_bench_100_devin2_head_release_90_update_dependencies_from_dotnet_arcade_pr80
Open

[release/9.0] Update dependencies from dotnet/arcade#12
hila-f-qodo wants to merge 7 commits intodevin_pr_code_review_bench_100_devin2_base_release_90_update_dependencies_from_dotnet_arcade_pr80from
devin_pr_code_review_bench_100_devin2_head_release_90_update_dependencies_from_dotnet_arcade_pr80

Conversation

@hila-f-qodo
Copy link
Copy Markdown

@hila-f-qodo hila-f-qodo commented Jan 26, 2026

Benchmark PR from qodo-benchmark#80


Open with Devin

dotnet-maestro Bot and others added 7 commits January 21, 2026 15:59
…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
…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
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View issue and 6 additional flags in Devin Review.

Open in Devin Review

Comment on lines +108 to +109
ParseErrorMessageRegexOld.IsMatch(logs[0].Message) &&
ParseErrorMessageRegexNew.IsMatch(logs[0].Message),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Incorrect logical operator in regex assertion - uses AND instead of OR

The assertion logic uses && (AND) when it should use || (OR) to match the error message.

Click to expand

Problem

The code at lines 107-110 asserts:

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}");

The error message says "match one of the CSP error patterns" but the code requires both patterns to match (&&). This is logically incorrect because:

  • ParseErrorMessageRegexOld matches: "Refused to frame...because an ancestor violates..."
  • ParseErrorMessageRegexNew matches: "Framing...violates..."

These are two different error message formats - a browser will produce one OR the other, never both simultaneously in the same message.

Impact

The test will always fail because a single log message cannot match both regex patterns at the same time. The test should pass if either pattern matches.

Expected behavior

The assertion should use || (OR):

Assert.True(
    ParseErrorMessageRegexOld.IsMatch(logs[0].Message) ||
    ParseErrorMessageRegexNew.IsMatch(logs[0].Message),
    ...);

Recommendation: Change && to || on line 108 so the assertion passes when either regex pattern matches the log message.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants