Implement dotnet test help option

[mariam-abdulla]

No description provided.

[MarcoRossignoli]

Did review round some question and cleanup

[mariam-abdulla]

@dotnet/domestic-cat We are using $(MicrosoftNETBuildTasksAssembly) in Microsoft.TestPlatform.ImportAfter.targets
but seems to be problematic in tests. Can you please recommend what can be done in that case?

[Forgind]

@dotnet/domestic-cat We are using $(MicrosoftNETBuildTasksAssembly) in Microsoft.TestPlatform.ImportAfter.targets but seems to be problematic in tests. Can you please recommend what can be done in that case?

That seems to be defined in Microsoft.NET.Sdk.Common.targets, which only comes in if you either import it directly or reference it indirectly by, for instance, using Sdk="Microsoft.NET.Sdk". I looked at one of the failing tests, and it was not an SDK-style project, and it didn't import that .targets file directly, which explains why it evaluated to "". You should be able to change the test asset to import it (and make sure you import it before Microsoft.TestPlatform.ImportAfter.targets).

Do you need this task in legacy .*proj files?

[mariam-abdulla]

@dotnet/domestic-cat We are using $(MicrosoftNETBuildTasksAssembly) in Microsoft.TestPlatform.ImportAfter.targets but seems to be problematic in tests. Can you please recommend what can be done in that case?

That seems to be defined in Microsoft.NET.Sdk.Common.targets, which only comes in if you either import it directly or reference it indirectly by, for instance, using Sdk="Microsoft.NET.Sdk". I looked at one of the failing tests, and it was not an SDK-style project, and it didn't import that .targets file directly, which explains why it evaluated to "". You should be able to change the test asset to import it (and make sure you import it before Microsoft.TestPlatform.ImportAfter.targets).

Do you need this task in legacy .*proj files?

When you say legacy, do you mean non SDK-style projects?

[Forgind]

Yes

[mariam-abdulla]

Yes

Then, we don't need the task there.

[Forgind]

Yes

Then, we don't need the task there.

Perfect, so we would just need to fix up the tests to make sure we only need to have that property defined in sdk-style test projects. Let me know if you need any help with that!

[mariam-abdulla]

Perfect, so we would just need to fix up the tests to make sure we only need to have that property defined in sdk-style test projects. Let me know if you need any help with that!

Yes, please let me know what should be done in that case.

[Forgind]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 4 pipeline(s).

[MarcoRossignoli]

@Forgind do we have some ETA to merge this first part of @mariam-abdulla PR? We need it to move to the next and unblock the task.

[MarcoRossignoli]

LGTM and tests are green now @Forgind @baronfel are you good for the merge, can you review?

For now we went for "manual" resolution of Microsoft.NET.Build.Tasks.dll https://github.com/dotnet/sdk/pull/41142/files#diff-3f7b2597025eca7c8bdaa8632fab1a3a6894415a704c2b08c2b0760ac708ba39 to fix the test failure
And added the cross targeting part close to the other one https://github.com/dotnet/sdk/pull/41142/files#diff-177f9060fda1f47c96d4e08705bbaab76098d76df09f8911eeea06586db65fc6

[Forgind]

I haven't really reviewed this properly, so I'll leave it to @baronfel and @MiYanni (who is now domestic-cat) to decide when to merge.

[baronfel]

LGTM - the main thing was the Task location and that's been addressed. Thanks folks, excited to see how this evolves.

[baronfel]

@MarcoRossignoli / @mariam-abdulla please feel free to merge whenever you like - consider squashing though since many of the individual commits in this PR aren't meaningful to the end result.