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

Do we need Microsoft.NET.Test.Sdk package to run tests? #986

Closed
srivatsn opened this Issue Aug 7, 2017 · 13 comments

Comments

Projects
None yet
4 participants
@srivatsn

srivatsn commented Aug 7, 2017

From @alrz on August 3, 2017 20:27

This is requested by Rider, I wonder if that is not the case for VS/RS.

Copied from original issue: dotnet/roslyn#21313

@srivatsn

This comment has been minimized.

Show comment
Hide comment
@srivatsn

srivatsn Aug 7, 2017

From @sharwell on August 3, 2017 20:41

I'm not sure I understand the question.

The main items added by Microsoft.NET.Test.Sdk are the UnitTestContainer capability and setting the IsTestProject property. The former is supposed to be the trigger for tests, but several older test adapter implementations do not add it. Roslyn does add it, so the Microsoft.NET.Test.Sdk package should not be needed for an IDE to know which projects are test projects and which ones are not.

srivatsn commented Aug 7, 2017

From @sharwell on August 3, 2017 20:41

I'm not sure I understand the question.

The main items added by Microsoft.NET.Test.Sdk are the UnitTestContainer capability and setting the IsTestProject property. The former is supposed to be the trigger for tests, but several older test adapter implementations do not add it. Roslyn does add it, so the Microsoft.NET.Test.Sdk package should not be needed for an IDE to know which projects are test projects and which ones are not.

@srivatsn

This comment has been minimized.

Show comment
Hide comment
@srivatsn

srivatsn Aug 7, 2017

From @alrz on August 3, 2017 20:45

I've found this #515 I think that's the issue I'm encountering.

srivatsn commented Aug 7, 2017

From @alrz on August 3, 2017 20:45

I've found this #515 I think that's the issue I'm encountering.

@srivatsn

This comment has been minimized.

Show comment
Hide comment
@srivatsn

srivatsn Aug 7, 2017

From @Pilchie on August 7, 2017 22:8

@ManishJayaswal or @srivatsn - do you know the right home for this?

srivatsn commented Aug 7, 2017

From @Pilchie on August 7, 2017 22:8

@ManishJayaswal or @srivatsn - do you know the right home for this?

@srivatsn

This comment has been minimized.

Show comment
Hide comment
@srivatsn

srivatsn Aug 7, 2017

Yes the test SDK does a few things like generate a Main method to keep the compiler happy and sets a few other things. There was an investigation about what we can do to either remove the requirement or make the Test.Sdk a real MSBuild SDK instead of a package. Moving to Microsoft/vstest

srivatsn commented Aug 7, 2017

Yes the test SDK does a few things like generate a Main method to keep the compiler happy and sets a few other things. There was an investigation about what we can do to either remove the requirement or make the Test.Sdk a real MSBuild SDK instead of a package. Moving to Microsoft/vstest

@Faizan2304

This comment has been minimized.

Show comment
Hide comment
@Faizan2304

Faizan2304 Aug 8, 2017

Contributor

@alrz
Microsoft.NET.Test.Sdk does lots of things. Most of the things are already covered by @srivatsn. One very important work of Microsoft.NET.Test.Sdk is that it takes the dependency of nuget package Microsoft.TestPlatform.TestHost which is needed to run the test. Without this test will not run for framework netcoreapp1.x.
We are creating Microsoft.NET.Test.Sdk this way because we want to bundle all the necessary things which are needed to run test in one place.

You can take individual nuget reference of Microsoft.TestPlatform.TestHost and run tests but it will be better if you will take nuget reference of Microsoft.NET.Test.Sdk

Contributor

Faizan2304 commented Aug 8, 2017

@alrz
Microsoft.NET.Test.Sdk does lots of things. Most of the things are already covered by @srivatsn. One very important work of Microsoft.NET.Test.Sdk is that it takes the dependency of nuget package Microsoft.TestPlatform.TestHost which is needed to run the test. Without this test will not run for framework netcoreapp1.x.
We are creating Microsoft.NET.Test.Sdk this way because we want to bundle all the necessary things which are needed to run test in one place.

You can take individual nuget reference of Microsoft.TestPlatform.TestHost and run tests but it will be better if you will take nuget reference of Microsoft.NET.Test.Sdk

@alrz

This comment has been minimized.

Show comment
Hide comment
@alrz

alrz Aug 8, 2017

so, shouldn't we add it to Roslyn? if not how can I workaround this for now (besides referencing it from all test projects)?

alrz commented Aug 8, 2017

so, shouldn't we add it to Roslyn? if not how can I workaround this for now (besides referencing it from all test projects)?

@sharwell

This comment has been minimized.

Show comment
Hide comment
@sharwell

sharwell Aug 8, 2017

Member

@alrz It's still not clear to me what the benefit would be if we add it to Roslyn. Can you describe a developer scenario which currently fails but would work if we referenced that package?

Member

sharwell commented Aug 8, 2017

@alrz It's still not clear to me what the benefit would be if we add it to Roslyn. Can you describe a developer scenario which currently fails but would work if we referenced that package?

@alrz

This comment has been minimized.

Show comment
Hide comment
@alrz

alrz Aug 10, 2017

@sharwell

image

I get this in Rider. This was not shown in the past. I didn't check yet if this is required in older commits.

alrz commented Aug 10, 2017

@sharwell

image

I get this in Rider. This was not shown in the past. I didn't check yet if this is required in older commits.

@sharwell

This comment has been minimized.

Show comment
Hide comment
@sharwell

sharwell Aug 10, 2017

Member

@alrz Ah. This was caused by dotnet/roslyn#20569. The solution is dotnet/roslyn#21017, but it's blocked due to some performance concerns that need to be thoroughly tested to make sure we don't break developers working on Roslyn.

Member

sharwell commented Aug 10, 2017

@alrz Ah. This was caused by dotnet/roslyn#20569. The solution is dotnet/roslyn#21017, but it's blocked due to some performance concerns that need to be thoroughly tested to make sure we don't break developers working on Roslyn.

@alrz

This comment has been minimized.

Show comment
Hide comment
@alrz

alrz Aug 10, 2017

@sharwell

I merged dotnet/roslyn#21017 locally but the issue remains -- Rider, R# and VS "Run Tests" are unable to discover any tests. I have no idea whether this belongs to either of IDEs or some test config/dependency.

alrz commented Aug 10, 2017

@sharwell

I merged dotnet/roslyn#21017 locally but the issue remains -- Rider, R# and VS "Run Tests" are unable to discover any tests. I have no idea whether this belongs to either of IDEs or some test config/dependency.

@sharwell

This comment has been minimized.

Show comment
Hide comment
@sharwell

sharwell Aug 10, 2017

Member

@alrz After merging it, you'll need to re-run the restore script and then rebuild the entire solution. After that things should be working again.

Member

sharwell commented Aug 10, 2017

@alrz After merging it, you'll need to re-run the restore script and then rebuild the entire solution. After that things should be working again.

@Faizan2304

This comment has been minimized.

Show comment
Hide comment
@Faizan2304

Faizan2304 Aug 14, 2017

Contributor

@alrz : Did you get your answer? Can i close this issue?

Contributor

Faizan2304 commented Aug 14, 2017

@alrz : Did you get your answer? Can i close this issue?

@alrz

This comment has been minimized.

Show comment
Hide comment
@alrz

alrz Aug 14, 2017

I no longer use IDE to run tests so this discussion is moot. Feel free to close the issue.

alrz commented Aug 14, 2017

I no longer use IDE to run tests so this discussion is moot. Feel free to close the issue.

@Faizan2304 Faizan2304 closed this Aug 14, 2017

Mpdreamz added a commit to elastic/elasticsearch-net that referenced this issue Sep 11, 2017

Alternative StartupObject workaround
Microsoft.Test.Sdk feels like injecting a Main method for some reason.

I tried to read up why it does so or why this package is even needed:

Microsoft/vstest#986

has something to do with magically supporting netcoreapp 1.1 targets but
i dread reading more. This is the sort of thing that tires me so much
doing .NET development..

The whole point this dependency is even needed infuriates me.

Mpdreamz added a commit to elastic/elasticsearch-net that referenced this issue Oct 21, 2017

Alternative StartupObject workaround
Microsoft.Test.Sdk feels like injecting a Main method for some reason.

I tried to read up why it does so or why this package is even needed:

Microsoft/vstest#986

has something to do with magically supporting netcoreapp 1.1 targets but
i dread reading more. This is the sort of thing that tires me so much
doing .NET development..

The whole point this dependency is even needed infuriates me.

Mpdreamz added a commit to elastic/elasticsearch-net that referenced this issue Oct 21, 2017

Alternative StartupObject workaround
Microsoft.Test.Sdk feels like injecting a Main method for some reason.

I tried to read up why it does so or why this package is even needed:

Microsoft/vstest#986

has something to do with magically supporting netcoreapp 1.1 targets but
i dread reading more. This is the sort of thing that tires me so much
doing .NET development..

The whole point this dependency is even needed infuriates me.

Mpdreamz added a commit to elastic/elasticsearch-net that referenced this issue Nov 16, 2017

Alternative StartupObject workaround
Microsoft.Test.Sdk feels like injecting a Main method for some reason.

I tried to read up why it does so or why this package is even needed:

Microsoft/vstest#986

has something to do with magically supporting netcoreapp 1.1 targets but
i dread reading more. This is the sort of thing that tires me so much
doing .NET development..

The whole point this dependency is even needed infuriates me.

Mpdreamz added a commit to elastic/elasticsearch-net that referenced this issue Nov 16, 2017

Alternative StartupObject workaround
Microsoft.Test.Sdk feels like injecting a Main method for some reason.

I tried to read up why it does so or why this package is even needed:

Microsoft/vstest#986

has something to do with magically supporting netcoreapp 1.1 targets but
i dread reading more. This is the sort of thing that tires me so much
doing .NET development..

The whole point this dependency is even needed infuriates me.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment