Skip to content
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

Read dotnet.config file to use MTP or VSTest in dotnet test #46717

Merged
merged 4 commits into from
Feb 12, 2025

Conversation

mariam-abdulla
Copy link
Member

@mariam-abdulla mariam-abdulla commented Feb 11, 2025

This pull request includes significant changes to the TestCommandParser class in the dotnet-test command. The primary focus of these changes is to improve the configuration handling and test runner determination logic.

Configuration handling improvements:

Test runner determination logic:

Relates to #45927

@mariam-abdulla mariam-abdulla requested a review from a team as a code owner February 11, 2025 10:53
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-dotnet test untriaged Request triage from a team member labels Feb 11, 2025
@KalleOlaviNiemitalo
Copy link
Contributor

Was there a decision to call the INI file dotnet.config? *.config files used with System.Configuration have XML format, so if you're going to use INI format here, then I'd prefer naming the file dotnet.ini or dotnet-config.ini.

@mariam-abdulla mariam-abdulla changed the title Add logic to use mtp or vstest in dotnet Read dotnet.config file to decide to use mtp or vstest in dotnet Feb 11, 2025
@mariam-abdulla mariam-abdulla changed the title Read dotnet.config file to decide to use mtp or vstest in dotnet Read dotnet.config file to use MTP or VSTest in dotnet test Feb 11, 2025
@Evangelink
Copy link
Member

The file name and format are still under discussion but as we need to move forward to allow internal dogfooding it was decided to temporarly move on with that (cc @baronfel )

Comment on lines +186 to 200
private static string? GetDotnetConfigPath(string? startDir)
{
string? directory = startDir;
while (directory != null)
{
string dotnetConfigPath = Path.Combine(directory, "dotnet.config");
if (File.Exists(dotnetConfigPath))
{
return dotnetConfigPath;
}

directory = Path.GetDirectoryName(directory);
}
return null;
}
Copy link
Contributor

@KalleOlaviNiemitalo KalleOlaviNiemitalo Feb 13, 2025

Choose a reason for hiding this comment

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

On Windows, should this disallow reading dotnet.config from the root directory of a volume, like #33216 and #33282 disallow reading .config\dotnet-tools.json from there?

Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose such a restriction is not necessary for security because the volume root directory on Windows typically has an NT AUTHORITY\Authenticated Users:(AD) access control entry. This ACE allows the user to create a subdirectory such as .config in the root directory, and the user can then get full control on the new subdirectory and create a dotnet-tools.json file in there. However, the ACE does not allow the user to create a file such as dotnet.config or global.json directly in the root directory. See FILE_ADD_SUBDIRECTORY in File Access Rights Constants.

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

Successfully merging this pull request may close these issues.

3 participants