Skip to content
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.

Sign up for GitHub

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

Jump to bottom

VSTest targeting netstandard 2.0 #2067

Merged
merged 33 commits into from
Jul 10, 2019
Merged

VSTest targeting netstandard 2.0 #2067

merged 33 commits into from
Jul 10, 2019

Conversation

vagisha-nidhi
Copy link
Contributor

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

Related issue: #2073

cltshivash
cltshivash reviewed Jun 27, 2019
View reviewed changes
scripts/build.ps1 Outdated Show resolved Hide resolved
@cltshivash
Copy link
Contributor 

}

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
Contributor

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
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
Contributor 

  <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
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
Contributor Author

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
Contributor Author

Choose a reason for hiding this comment

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

@singhsarab

Copy link
Contributor

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
Contributor Author 

<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
Copy link
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
Contributor Author 

<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
Contributor

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
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
Member

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
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
Member

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
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

@cltshivash cltshivash cltshivash left review comments

@singhsarab singhsarab singhsarab approved these changes

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