Project naming changes project behavior

[jcagme]

The Roslyn team’s opinions on how to name projects is now baked into Arcade, and it doesn’t fit aspnet’s opinions. E.g. ‘Something.IntegrationTests’ vs ‘Something.UnitTests’.

We are treating these projects differently, and it’s a pain to do something different. There are some MSBuild properties that can be overridden but looking at Arcade's code is need to reverse engineer what's actually done.

[tmat]

How are you treating them differently? What behavior do you want to change?

[jcagme]

@natemcmaster @dougbu

[natemcmaster]

AspNetCore doesn't...

• always use xunit
• split test runs between unit, integration, and performance based on project name
• always name test projects which match these conditions. Some tests deviate from these patterns https://github.com/dotnet/arcade/blob/9104c21492e1c6ed98105dd87388bca8f1f1abaa/src/Microsoft.DotNet.CoreFxTesting/build/TestProject.props#L9-L22

Also, AspNetCore, Extensions, and EF Core ship some packages to customers called 'Specifications.Tests'. These are mean to be tests in a class library which our customers use to write and test their own implementations of our abstractions. When converting to Arcade, we noticed the dependencies on these packages changing due to new implicit references. https://github.com/dotnet/arcade/blob/268dc10408d39b6059924989a071549371757fb1/src/Microsoft.DotNet.Arcade.Sdk/tools/XUnit/XUnit.targets#L9-L13

[natemcmaster]

The behavior I'd like to change is what the title of the issue says, "Project naming changes project behavior". I'm okay if repos want to opt-in to project naming conventions, but IMO that should be defined by the repo using Directory.Build.props/targets. It is heavy-handed for Arcade SDK to change references and test behavior based on the .csproj file name alone.

[markwilkie]

@natemcmaster - one of the goals of Arcade is to bring some consistency to our builds such that devs are completely "lost" when moving from one repo to another. Do you have any thoughts for how you see balancing that goal being "heavy handed"?

There certainly is a challenge here in that ASP builds very different. I'd like to focus in on the value and then look to see what the cheapest way we can get there is.

[tmat]

always use xunit

This is not Arcade requirement, just default. It is designed to support other test frameworks, we have just not had time to add NUnit and MSTest runner support. But the infrastructure is ready for it, just needs a corresponding target to be added.

split test runs between unit, integration, and performance based on project name

There is no requirement for using IntegrationTests and PerformanceTests distinction. All your test projects could be named '.Tests'

Also, AspNetCore, Extensions, and EF Core ship some packages to customers called 'Specifications.Tests'. These are mean to be tests in a class library which our customers use to write and test their own implementations of our abstractions. When converting to Arcade, we noticed the dependencies on these packages changing due to new implicit references.

There are two options here:

• You can rename the project file to Specifications.Tests.Library or something like that and set AssemblyName in that project explicitly to Specifications.Tests.
• We can allow projects to opt-out of the naming convention. This should be simple, just changing https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.Arcade.Sdk/tools/Tests.props#L25 <IsTestProject Condition="'$(IsTestProject)' == ''">false</IsTestProject>. You'd then set IsTestProject=false in Directory.Build.props before importing Arcade SDK props.

[natemcmaster]

bring some consistency to our builds such that devs are completely "lost" when moving from one repo to another. Do you have any thoughts for how you see balancing that goal being "heavy handed"?

I think you have to balance this against the way the rest of the entire .NET world uses MSBuild. AFAIK very few do things like adding implicit references based on naming alone. Implicit references are tricky and lead to problems like IMO this behavior adds more people to the "lost" group than it does to the "found"...specifically, our external contributors.

We can allow projects to opt-out of the naming convention.

Yes that would be useful. I was looking at this closer today because our packages have new dependencies as a result of converting to Arcade. We had set IsUnitTestProject=false in the .csproj, but that doesn't happen early enough to override the implicit references added by Arcade.

[markwilkie]

Moving into the 'defaults' discussion.

[sharwell]

@natemcmaster Roslyn also didn't follow the convention originally, but renamed many (all?) of its projects to follow the new convention when arcade was adopted. Part of the benefit of common infrastructure is that common infrastructure patterns are followed in a common way, even when causes a bit of adoption pain as a one-time thing.

[markwilkie]

Given the guidance now articulated in https://github.com/dotnet/arcade/blob/master/Documentation/DefaultsGuidance.md, @natemcmaster, what's your recommend for next steps?

[natemcmaster]

I recommend we accept this as a well-entrenched behavior of Arcade which can be overridden. At this point, so many repos have been changed to fit this convention that there is little value in altering course on this as a default behavior. It is not the default behavior I would have chosen in the beginning, but it's probably too late to change directions on this one, especially when there is still a strong tail-wind pushing us to get in line with other repos.