Revise TryGetProjectOrSolutionFilePath and GetSolutionFilePaths logic for the new dotnet test experience

[Youssef1313]

sdk/src/Cli/dotnet/commands/dotnet-test/SolutionAndProjectUtility.cs

Lines 87 to 88 in 9f73261

	string[] solutionFiles = Directory.GetFiles(directory, CliConstants.SolutionExtensionPattern, SearchOption.TopDirectoryOnly);
	solutionFiles.AddRange(Directory.GetFiles(directory, CliConstants.SolutionXExtensionPattern, SearchOption.TopDirectoryOnly));

The second line above should crash. An array is fixed-length. It's implicitly converted to ICollection<string> in that code path and should throw some exception at run-time.

---

sdk/src/Cli/dotnet/commands/dotnet-test/SolutionAndProjectUtility.cs

Lines 19 to 83 in 9f73261

	public static (bool SolutionOrProjectFileFound, string Message) TryGetProjectOrSolutionFilePath(string directory, out string projectOrSolutionFilePath, out bool isSolution)
	{
	projectOrSolutionFilePath = string.Empty;
	isSolution = false;
	if (!Directory.Exists(directory))
	{
	return (false, string.Format(LocalizableStrings.CmdNonExistentDirectoryErrorDescription, directory));
	}
	var solutionPaths = GetSolutionFilePaths(directory);
	// If more than a single sln file is found, an error is thrown since we can't determine which one to choose.
	if (solutionPaths.Length > 1)
	{
	return (false, string.Format(CommonLocalizableStrings.MoreThanOneSolutionInDirectory, directory));
	}
	if (solutionPaths.Length == 1)
	{
	var projectPaths = GetProjectFilePaths(directory);
	if (projectPaths.Length == 0)
	{
	projectOrSolutionFilePath = solutionPaths[0];
	isSolution = true;
	return (true, string.Empty);
	}
	return (false, LocalizableStrings.CmdMultipleProjectOrSolutionFilesErrorDescription);
	}
	else // If no solutions are found, look for a project file
	{
	string[] projectPaths = GetProjectFilePaths(directory);
	if (projectPaths.Length == 0)
	{
	var solutionFilterPaths = GetSolutionFilterFilePaths(directory);
	if (solutionFilterPaths.Length == 0)
	{
	return (false, LocalizableStrings.CmdNoProjectOrSolutionFileErrorDescription);
	}
	if (solutionFilterPaths.Length == 1)
	{
	projectOrSolutionFilePath = solutionFilterPaths[0];
	isSolution = true;
	return (true, string.Empty);
	}
	else
	{
	return (false, LocalizableStrings.CmdMultipleProjectOrSolutionFilesErrorDescription);
	}
	}
	if (projectPaths.Length == 1)
	{
	projectOrSolutionFilePath = projectPaths[0];
	return (true, string.Empty);
	}
	return (false, string.Format(CommonLocalizableStrings.MoreThanOneSolutionInDirectory, directory));
	}
	}

I'm reading this logic as "If there are no sln/slnx files at all, and a single project file is found, use the project file". But what if the directory has slnf and project file, without sln/slnx? I think we should either prefer the slnf, or error. It's a pattern I have never seen, but ...

Anyways, we should validate the behavior of dotnet build, and make sure we match it, and share some common helpers if needed.

[mariam-abdulla]

For fixing GetSolutionFilePaths, good catch. I will fix it.

However, when we have both slnf and project file together in the same directory. We have the same behavior as msbuild which is considering the project file and ignoring the slnf.

[Youssef1313]

Interesting. @rainersigwald Are the rules for what to consider documented somewhere?

I'm wondering if some of the current behavior are only for backcompat reasons. Maybe we could consider some scenarios as "ambiguous" and do error for the new dotnet test experience as we don't really have backcompat concerns?

[Youssef1313]

I'm re-opening as the second part of this issue wasn't addressed. Ping @rainersigwald @baronfel for what rules we should follow here.

[Youssef1313]

Ping @rainersigwald

[Youssef1313]

It looks like the logic here should be more complicated if we want to match MSBuild. For example, when one project and one solution exists in the directory, the behavior differs depending on whether the file names (without the extension for sure) are the same or not.

https://github.com/dotnet/msbuild/blob/b878078fbaa28491a3a7fb273474ba71675c1613/src/MSBuild/XMake.cs#L3589-L3598

There is more logic around this piece of code as well.

[Youssef1313]

@dsplaisted Is there a logic in SDK already somewhere that matches MSBuild in this regard?

[dsplaisted]

@Youssef1313 It looks like we have that here:

sdk/src/Cli/dotnet/ReleasePropertyProjectLocator.cs

Line 107 in 99fe7ae

public ProjectInstance? GetTargetedProject(Dictionary<string, string> globalProps)

It doesn't look like it's written to be used from other code, so probably if you need this logic it should be factored out in order to be reusable.

[dsplaisted]

Actually, it looks like that function doesn't do exactly what you're looking for, it will return an arbitrary project in the solution if it locates a solution instead of returning the solution itself. But the functionality you are looking for could probably be factored out.

[Youssef1313]

Hmm, the logic you linked doesn't seem to match what I linked from MSBuild :/
Not sure then if the only way we have is to duplicate the logic from MSBuild?

I think we always want to ensure the two behaviors match exactly so that cases where people do

dotnet build
dotnet test --no-build

are guaranteed to work as expected (both build and test command operate on the exact same set of projects)

[surayya-MS]

I'm reading this logic as "If there are no sln/slnx files at all, and a single project file is found, use the project file". But what if the directory has slnf and project file, without sln/slnx? I think we should either prefer the slnf, or error. It's a pattern I have never seen, but ...

@Youssef1313 if there is no sln/slnx and there is only a project file and slnf then use project file. This is what we do in MSBuild
https://github.com/dotnet/msbuild/blob/b878078fbaa28491a3a7fb273474ba71675c1613/src/MSBuild/XMake.cs#L3644-L3650

[Youssef1313]

@surayya-MS Is there a logic exposed somewhere via a public API that we can use here? It will be a bit unfortunate to duplicate all the logic in MSBuild here.

[surayya-MS]

@Youssef1313 unfortunately, there is no public API for this. The only option for now is to implement the same logic on your side. We could potentially add this to public API in the future. That needs more discussion.
cc @rainersigwald