Upgrade to VS 2017 #734
Upgrade to VS 2017 #734
Conversation
| <PreserveCompilationContext>true</PreserveCompilationContext> | ||
| <OutputType>Exe</OutputType> | ||
| <WarningsAsErrors>true</WarningsAsErrors> | ||
| <RuntimeIdentifiers>win7-x64;win7-x86;win10-x64;win10-x86;osx.10.10-x64;osx.10.11-x64;osx.10.12-x64;ubuntu.14.04-x64;ubuntu.15.04-x64;centos.7-x64;rhel.7.2-x64</RuntimeIdentifiers> |
There was a problem hiding this comment.
add or replace? do we ever run tests on 15?
There was a problem hiding this comment.
I believe odd version numbers is something like a preview release, we can just add 16 and keep 15, doesn't hurt
samples/MusicStore/MusicStore.csproj
Outdated
| @@ -0,0 +1,22 @@ | |||
| <Project Sdk="Microsoft.NET.Sdk.Web" ToolsVersion="15.0"> | |||
|
|
|||
| <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), 'version.props'))\version.props" /> | |||
There was a problem hiding this comment.
Curious, is \ going to be fine on Unix systems?
|
With my limited knowledge it looks good |
|
Found a blocking issue! We can't upgrade this project yet because we won't have a way to filter tests based on Xunit trait. We need this to run tests for that are specifically designed for NanoServer. dotnet-test-xunit provided a way to filter on traits, but there is no equivalent in xunit.runner.visualstudio and dotnet-test for MSBuild. It looks like this issue has been opened many times, but there has been no resolution yet. |
| @@ -12,7 +13,12 @@ protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage reques | |||
| var response = new HttpResponseMessage(); | |||
|
|
|||
| var basePath = Path.GetFullPath(Path.Combine( | |||
| Directory.GetCurrentDirectory(), "ForTesting", "Mocks", "OpenIdConnect")); | |||
| #if NET451 | |||
There was a problem hiding this comment.
Curious - why would any code need to change as part of the migration?
There was a problem hiding this comment.
dotnet-test breaks on any tests that use Directory.GetCurrentDirectory(). See https://developercommunity.visualstudio.com/content/problem/7956/current-working-directory-of-net-core-tests-is-set.html
There was a problem hiding this comment.
Is this just a bug? That seems quite odd, no? I would imagine that tons of tests would break during migration (not just ours, but anyone's tests).
There was a problem hiding this comment.
It's a bug. The vstest team is tracking a fix for this here: microsoft/vstest#311
natemcmaster
force-pushed
the
namc/vs2017
branch
from
fa214b8
to
c4d0106
Compare
|
🆙 📅 Removing [blocked] label. I've added a workaround for the inability to filter by xunit traits. (see fe4bb03) Also, discovered that these tests weren't running anyways: aspnet/Testing#244 |
natemcmaster
force-pushed
the
namc/vs2017
branch
from
fe4bb03
to
2e1b7bd
Compare
natemcmaster
force-pushed
the
namc/vs2017
branch
from
2e1b7bd
to
b099e03
Compare
Part of aspnet/Universe#469.
Depends on aspnet/Hosting#913.
Includes a few workaround for RC.2 bugs, but should mostly work now.
✅ E2E tests pass.
✅ Launching/debugging MusicStore portable in VS works.
❌ Launching/debugging MusicStore.Standalone in VS fails. cc @mlorbetske
cc @muratg @Eilon