-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add the test verifying property functions used in dotnet console/webapp don't need a reflection #48475
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
Conversation
…pp dont need a reflection
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.
Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.
Files not reviewed (1)
- test/TestAssets/TestProjects/MSBuildBareBonesProject/BareBones.proj: Language not supported
Comments suppressed due to low confidence (2)
test/Microsoft.NET.Build.Tests/EvaluatorFastPathTests.cs:58
- Consider verifying that the log file exists before reading from it to prevent potential exceptions if the file is missing.
var lines = File.ReadAllLines(logPath);
test/Microsoft.NET.Build.Tests/EvaluatorFastPathTests.cs:28
- [nitpick] Consider extracting the log file name "PropertyFunctionsRequiringReflection" into a named constant to avoid duplication and potential discrepancies in future updates.
var logPath = Path.Combine(testAsset.Path, "PropertyFunctionsRequiringReflection");
[Theory] | ||
[InlineData("console")] | ||
[InlineData("webapp")] | ||
public void EnsureDotnetCommonProjectPropertyFunctionsOnFastPath(string alias) |
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.
public void EnsureDotnetCommonProjectPropertyFunctionsOnFastPath(string alias) | |
public void EnsureDotnetCommonProjectPropertyFunctionsOnFastPath(string alias_) |
alias
is a keyword so let's avoid it as a name for variable
{ | ||
var testDir = _testAssetsManager.CreateTestDirectory().Path; | ||
|
||
new DotnetNewCommand(Log, alias) |
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.
new DotnetNewCommand(Log, alias) | |
new DotnetNewCommand(Log, alias_) |
Fixes dotnet/msbuild#10406
ToolLocationHelper property functions still require reflection and it's tracked by dotnet/msbuild#10411. For other functions the test is covered.