Skip to content

VSTest targeting netstandard 2.0#2067

Merged
vagisha-nidhi merged 33 commits intomicrosoft:masterfrom
vagisha-nidhi:ns2.0
Jul 10, 2019
Merged

VSTest targeting netstandard 2.0#2067
vagisha-nidhi merged 33 commits intomicrosoft:masterfrom
vagisha-nidhi:ns2.0

Conversation

@vagisha-nidhi
Copy link
Copy Markdown
Contributor

@vagisha-nidhi vagisha-nidhi commented Jun 26, 2019 •
edited
Loading

Related issue: #2073

cltshivash
cltshivash reviewed Jun 27, 2019
View reviewed changes
cltshivash
cltshivash reviewed Jun 27, 2019
View reviewed changes
@cltshivash
Copy link
Copy Markdown
Contributor

cltshivash commented Jun 27, 2019 

}

can we remove this ?

Refers to: scripts/build.ps1:170 in f0e0007. [](commit_id = f0e0007, deletion_comment = False)

cltshivash
cltshivash reviewed Jun 27, 2019
View reviewed changes
src/Microsoft.TestPlatform.Client/Microsoft.TestPlatform.Client.csproj
<TargetFrameworks>netstandard1.4;net451</TargetFrameworks>
<TargetFrameworks Condition=" '$(DotNetBuildFromSource)' == 'true' ">netstandard1.4</TargetFrameworks>
<TargetFrameworks>netstandard2.0;net451</TargetFrameworks>
<TargetFrameworks Condition=" '$(DotNetBuildFromSource)' == 'true' ">netstandard2.0</TargetFrameworks>
Copy link
Copy Markdown
Contributor

@cltshivash cltshivash Jun 27, 2019

Choose a reason for hiding this comment

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

can this move into the TestPlatform.settings.targets ? Will prevent adding for every project

@cltshivash
Copy link
Copy Markdown
Contributor

cltshivash commented Jun 27, 2019 •
edited by vagisha-nidhi
Loading 

<Reference Include="Microsoft.CSharp" />

can we check why this differentiation is needed ?

Refers to: src/Microsoft.TestPlatform.CommunicationUtilities/Microsoft.TestPlatform.CommunicationUtilities.csproj:32 in f0e0007. [](commit_id = f0e0007, deletion_comment = False)

@cltshivash
Copy link
Copy Markdown
Contributor

cltshivash commented Jun 27, 2019 

  <Version>4.0.1</Version>

will these versions hold good or needs to be bumped up as well ?

Refers to: src/Microsoft.TestPlatform.Extensions.TrxLogger/Microsoft.TestPlatform.Extensions.TrxLogger.csproj:34 in f0e0007. [](commit_id = f0e0007, deletion_comment = False)

@mayankbansal018 mayankbansal018 mentioned this pull request Jul 1, 2019
Closed
vagisha-nidhi
vagisha-nidhi commented Jul 8, 2019
View reviewed changes
vagisha-nidhi
vagisha-nidhi commented Jul 8, 2019
View reviewed changes
src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj
</PackageReference>
<PackageReference Include="System.Reflection.TypeExtensions">
<Version>4.1.0</Version>
<Version>1.6.0</Version>
Copy link
Copy Markdown
Contributor Author

@vagisha-nidhi vagisha-nidhi Jul 8, 2019

Choose a reason for hiding this comment

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

1.6.0 [](start = 15, length = 5)

Do we bump up this version or remain at 1.3.0?

Copy link
Copy Markdown
Contributor Author

@vagisha-nidhi vagisha-nidhi Jul 8, 2019

Choose a reason for hiding this comment

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

@singhsarab

Copy link
Copy Markdown
Contributor

@singhsarab singhsarab Jul 9, 2019

Choose a reason for hiding this comment

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

Keep it at 1.3.0 if it has netstandard2.0 support

In reply to: 301028228 [](ancestors = 301028228)

@vagisha-nidhi
Copy link
Copy Markdown
Contributor Author

vagisha-nidhi commented Jul 8, 2019 

<Reference Include="Microsoft.CSharp" />

Check if needed

Refers to: src/Microsoft.TestPlatform.TestHostProvider/Microsoft.TestPlatform.TestHostProvider.csproj:35 in ba48652. [](commit_id = ba48652, deletion_comment = True)

vagisha-nidhi
vagisha-nidhi commented Jul 8, 2019
View reviewed changes
vagisha-nidhi
vagisha-nidhi commented Jul 8, 2019
View reviewed changes
@vagisha-nidhi
Copy link
Copy Markdown
Contributor Author

vagisha-nidhi commented Jul 8, 2019 •
edited
Loading

�

?? Please check this

Refers to: test/TestAssets/SimpleTestProjectx86/SimpleTestProjectx86.csproj:12 in 4964d3b. [](commit_id = 4964d3b, deletion_comment = False)

@singhsarab Seems normal when opened in VS.

@vagisha-nidhi vagisha-nidhi changed the title [Draft] VSTest targeting netstandard 2.0 VSTest targeting netstandard 2.0 Jul 8, 2019
@vagisha-nidhi
Copy link
Copy Markdown
Contributor Author

vagisha-nidhi commented Jul 9, 2019 

<Reference Include="Microsoft.CSharp" />

Check if needed

Refers to: src/Microsoft.TestPlatform.TestHostProvider/Microsoft.TestPlatform.TestHostProvider.csproj:35 in ba48652. [](commit_id = ba48652, deletion_comment = True)

Will take this up with net451 changes

singhsarab
singhsarab reviewed Jul 9, 2019
View reviewed changes
src/vstest.console/Processors/ResultsDirectoryArgumentProcessor.cs Outdated
throw new ArgumentException("Illegal characters in path.");
}

var di = new FileInfo(argument);
Copy link
Copy Markdown
Contributor

@singhsarab singhsarab Jul 9, 2019

Choose a reason for hiding this comment

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

FileInfo [](start = 29, length = 8)

Keep it DirectoryInfo

singhsarab
singhsarab approved these changes Jul 9, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@singhsarab singhsarab left a comment

Choose a reason for hiding this comment

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

Approving with suggestions.

Please do make sure you have made all the changes requested. Removed netstandard1.4 references from test assets as well.

@singhsarab singhsarab mentioned this pull request Jul 9, 2019
Closed
@vagisha-nidhi vagisha-nidhi merged commit 9f0de74 into microsoft:master Jul 10, 2019
@singhsarab singhsarab mentioned this pull request Jul 29, 2019
Closed
JSkimming added a commit to JSkimming/Castle.Core.AsyncInterceptor that referenced this pull request Sep 20, 2019
@JSkimming
Remove the netstandard1.4 build target
4e3a300 
It will become increasingly difficult to maintain since the tests can no longer target netcoreapp1.1 since microsoft/vstest#2067
@JSkimming JSkimming mentioned this pull request Sep 20, 2019
Merged
@jnyrup jnyrup mentioned this pull request Sep 21, 2019
Closed
@sharwell
Copy link
Copy Markdown
Contributor

sharwell commented Jul 29, 2020

This is super unfortunate. We have projects targeting netstandard1.x (test running on netcoreapp1.1), and this change means we can never update Microsoft.NET.Test.Sdk to any version newer than 16.1.1.

sharwell added a commit to dotnet/roslyn-sdk that referenced this pull request Jul 29, 2020
@sharwell
Lock to prior version of Microsoft.NET.Test.Sdk
ce83a27 
Support for netcoreapp1.1 was removed in microsoft/vstest#2067 and
microsoft/vstest#2192, so we can no longer take updates to this package.
dotnet-maestro bot added a commit to dotnet/roslyn-sdk that referenced this pull request Jul 29, 2020
@dotnet-maestro
[master] Update dependencies from dotnet/arcade (#574)
e2e9d63 
[master] Update dependencies from dotnet/arcade
- Updates:
  - Microsoft.DotNet.Arcade.Sdk: from 5.0.0-beta.20117.3 to 5.0.0-beta.20374.1

 - Upadte xcopy-msbuild

 - Switch to Arcade UnitTest targets and props

 - Install the 3.1.0 runtime for test running

 - Use default test versions from Arcade

 - Work around implicit package reference warning

 - Set DOTNET_ROOT variable

 - Remove not working workaround for implicit package reference

 - Reference the XUnit Assert package

 - Lock to prior version of Microsoft.NET.Test.Sdk

Support for netcoreapp1.1 was removed in microsoft/vstest#2067 and
microsoft/vstest#2192, so we can no longer take updates to this package.
@nohwnd
Copy link
Copy Markdown
Member

nohwnd commented Jul 30, 2020

@sharwell Is it not possible to update to netcoreapp2.1, or any of the supported .NET Core versions?

@sharwell
Copy link
Copy Markdown
Contributor

sharwell commented Jul 30, 2020 •
edited
Loading

@nohwnd We use netcoreapp2.1 for testing our netstandard2.0 build. netcoreapp1.1 is a better representation of our intended support scenario.

@nohwnd
Copy link
Copy Markdown
Member

nohwnd commented Jul 31, 2020

I am afraid we can't help you there 🙁 I suspect downgrading testhost to netcoreapp1.1 would be a significant effort. /cc @pavelhorak

@sharwell sharwell mentioned this pull request Sep 1, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@cltshivash cltshivash cltshivash left review comments

@singhsarab singhsarab singhsarab approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vagisha-nidhi @cltshivash @singhsarab @sharwell @nohwnd