Upgrade to VS 2017 #194

natemcmaster · 2017-02-03T01:17:52Z

Issues:

had to disable tests that run standalone apps because of Runtime-specific build fails for projects with P2P references dotnet/sdk#696
add xunit output logger

TODO:

~~Tag helper tests are failing~~ Resolved.

Failed   EntropyTests.TagHelperSampleTest.MoviesController_Pages_ReturnSuccess(requestPages: Func`2 { Method = System.Threading.Tasks.Task`1[System.Collections.Generic.IEnumerable`1[System.Net.Http.HttpResponseMessage]] <get_MoviesControllerPageData>b__12_0(System.Net.Http.HttpClient), Target = <>c { } })
Error Message:
 Microsoft.AspNetCore.Mvc.Razor.Compilation.CompilationFailedException : One or more compilation failures occurred:
Error @ (612:11,80)(30) - [The assembly 'Microsoft.AspNetCore.Mvc.Razor' could not be resolved or contains no tag helpers.] (11,80) The assembly 'Microsoft.AspNetCore.Mvc.Razor' could not be resolved or contains no tag helpers.
Error @ (17:0,17)(35) - [The assembly 'Microsoft.AspNetCore.Mvc.TagHelpers' could not be resolved or contains no tag helpers.] (0,17) The assembly 'Microsoft.AspNetCore.Mvc.TagHelpers' could not be resolved or contains no tag helpers.

The test app that is being run builds, publishes, and runs just fine.

Any ideas @NTaylorMullen ?

Resolves #193

dnfclas · 2017-02-03T01:17:55Z

Hi @natemcmaster, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

NTaylorMullen · 2017-02-03T01:20:23Z

Any ideas @NTaylorMullen ?

Is that only failing on desktop?

natemcmaster · 2017-02-03T02:12:45Z

Actually, the entire .NET Core test run is failing:

Test run for C:\projects\entropy\test\FunctionalTests\bin\Debug\netcoreapp1.1\FunctionalTests.dll(.NETCoreApp,Version=v1.1)
Microsoft (R) Test Execution Command Line Tool Version 15.0.0.0
Copyright (c) Microsoft Corporation. All rights reserved.
Starting test execution, please wait...
Warning: No test is available in C:\projects\entropy\test\FunctionalTests\bin\Debug\netcoreapp1.1\FunctionalTests.dll. Make sure that installed test discoverers & executors, platform & framework version settings are appropriate and try again.

The error I mentioned is only desktop...as far as I can tell

NTaylorMullen · 2017-02-03T18:49:44Z

Try disabling shadow copy on test runs. The TagHelper Assembly.Load is probably failing due to it.

natemcmaster · 2017-02-03T19:37:40Z

I tried that. Still fails 😕

NTaylorMullen · 2017-02-03T20:06:51Z

Actually, the entire .NET Core test run is failing:

Grab the latest vstest bits and update your host locally. It'll split out more information as to why it's failing to find tests.

natemcmaster · 2017-02-03T23:48:01Z

I have all but 16 tests passing now. On both .NET Framework and .NET Core, all failing tests have this error:

CompilationFailedException : One or more compilation failures occurred:
Error @ (612:11,80)(30) - [The assembly 'Microsoft.AspNetCore.Mvc.Razor' could not be resolved or contains no tag helpers.] (11,80) The assembly 'Microsoft.AspNetCore.Mvc.Razor' could not be resolved or contains no tag helpers.

NTaylorMullen · 2017-02-03T23:51:28Z

Did it pass in project.json land? I'm curious if @rynowak's recent conversion to use new Razor caused this.

natemcmaster · 2017-02-04T00:01:32Z

Yes, it passes in project.json. Does razor rely on Assembly.GetEntryAssembly to locate anything? This is one of the things that changed with xunit.runner.visualstudio from dotnet-test-xunit

NTaylorMullen · 2017-02-04T00:09:35Z

I believe we indirectly depend on it via DependencyContext; however, didn't old world also mess up GetEntryAssembly too?

I'm not familiar with how Entropy runs their tests but if it's anything like MVC you may need to do the following to ensure DependencyContext.Load works: https://github.com/aspnet/Mvc/pull/5749/files#diff-c611e174495fa124b795bc0138d9bf50R52

natemcmaster · 2017-02-04T00:18:04Z

Brilliant! That little target to copy deps files fixed it. I'll add to these functional tests. Do you have an issue tracking the issue the removal of this workaround?

natemcmaster · 2017-02-04T00:20:04Z

Remove wip label. Should be ready to merge now.

NTaylorMullen · 2017-02-04T00:42:13Z

Brilliant! That little target to copy deps files fixed it. I'll add to these functional tests. Do you have an issue tracking the issue the removal of this workaround?

Not a workaround. It was a by design break. Before it would embed the deps.json in each of the dependent dll's so DependencyContext.Load would just work; now it places the deps.json next to the dll in its own output dir (which is why we need to copy them over).

natemcmaster · 2017-02-04T00:59:45Z

Oh. How many people are actually doing functional tests with razor? Won't they all hit this now?

We should be able to revert this entire commit when external issues are fixed

NTaylorMullen · 2017-02-04T00:52:54Z

...Config.CustomConfigurationProviders.Sample/Config.CustomConfigurationProviders.Sample.csproj

@@ -0,0 +1,21 @@
+<Project Sdk="Microsoft.NET.Sdk">


Not Sdk.Web?

It's at actually not a web project. It only has web.config because it's showing how to use configuration to read xml.

NTaylorMullen · 2017-02-04T00:55:37Z

...Config.CustomConfigurationProviders.Sample/Config.CustomConfigurationProviders.Sample.csproj

@@ -0,0 +1,21 @@
+<Project Sdk="Microsoft.NET.Sdk">


Ditto question about the sdk.web. Mostly just asking because of the Content include on web.config

NTaylorMullen · 2017-02-04T00:58:25Z

samples/Mvc.AjaxAntiforgeryAuth.Web/.gitignore

@@ -0,0 +1 @@
+wwwroot/lib/


Curious: Why'd need to be added in the new world?

nothing changed. I was just tired of seeing a bunch of files installed from bower as pending changes in git.

NTaylorMullen · 2017-02-04T00:59:42Z

samples/Mvc.FileUpload/Mvc.FileUpload.csproj

+  </PropertyGroup>
+
+  <ItemGroup>
+    <Content Update="wwwroot\**\*;Views\**\*;appsettings.json;web.config" CopyToOutputDirectory="PreserveNewest" />


Can remove 😄

NTaylorMullen · 2017-02-04T00:59:51Z

samples/Mvc.FileUpload/Mvc.FileUpload.csproj

+  </ItemGroup>
+
+  <ItemGroup>
+    <DotNetCliToolReference Include="Microsoft.DotNet.Watcher.Tools" Version="1.0.0-msbuild3-final" />


NTaylorMullen · 2017-02-04T01:05:45Z

test/FunctionalTests/FunctionalTests.csproj

+  </PropertyGroup>
+
+  <ItemGroup>
+    <EmbeddedResource Include="resources\*.*" Exclude="bin\**;obj\**;**\*.xproj;packages\**;@(EmbeddedResource)" />


Need the excludes?

NTaylorMullen · 2017-02-04T01:06:10Z

test/FunctionalTests/FunctionalTests.csproj

+    <TargetFrameworks>netcoreapp1.1;net451</TargetFrameworks>
+    <DefineConstants>$(DefineConstants);__RemoveThisBitTo__GENERATE_BASELINES</DefineConstants>
+    <!-- set explicitly because some tests depend on this -->
+    <RootNamespace>FunctionalTests</RootNamespace>


Affects the name of the embedded resources.

NTaylorMullen · 2017-02-04T01:16:30Z

test/FunctionalTests/XunitLogger.cs

+        private readonly string _categoryName;
+
+        public XunitLogger(ITestOutputHelper output, string categoryName)
+        {;


lawl semi-colon

NTaylorMullen · 2017-02-04T01:16:55Z

test/FunctionalTests/XunitLogger.cs

+
+namespace EntropyTests
+{
+    public class XunitLogger : ILogger, IDisposable


This might not be needed with the latest XUnit. They're much more verbose.

Still helps us a lot because the tests that deploy a server produce lots of console output. It's better to have this captured by xunit since console.writeline interleaves different tests.

NTaylorMullen · 2017-02-04T01:17:04Z

test/FunctionalTests/XunitLoggerProvider.cs

+
+namespace EntropyTests
+{
+    class XunitLoggerProvider : ILoggerProvider


public is your friend

NTaylorMullen · 2017-02-04T01:18:17Z

Oh. How many people are actually doing functional tests with razor? Won't they all hit this now?

Yup, it's an odd way to do it though. We're taking a dependency on sample apps and then testing them in memory instead of individually publishing them in their own context.

natemcmaster · 2017-02-04T01:19:35Z

⌚️ on breaking changes from localization to make it through CI

natemcmaster · 2017-02-04T02:25:25Z

Passes locally, not on CI for reasons I don't understand. Will check again on Monday.

NTaylorMullen · 2017-02-06T20:10:37Z

test/FunctionalTests/FunctionalTests.csproj

+  <ItemGroup>
+    <EmbeddedResource Include="resources\**\*" />
+    <Content Include="nginx.conf" CopyToOutputDirectory="PreserveNewest" />
+    <None Include="xunit.runner.json" CopyToOutputDirectory="PreserveNewest" />


Good to know about this guy. I didn't 😄

natemcmaster · 2017-02-06T20:53:36Z

Finally passing on Windows.

natemcmaster · 2017-02-06T21:31:05Z

🔔 tests passing, ready to merge.

NTaylorMullen · 2017-02-06T22:41:01Z

As long as its usable in VS.

dnfclas added the cla-already-signed label Feb 3, 2017

natemcmaster changed the title ~~wip: Upgrade to VS 2017~~ Upgrade to VS 2017 Feb 4, 2017

natemcmaster requested a review from NTaylorMullen February 4, 2017 00:20

Nate McMaster added 4 commits February 3, 2017 17:06

Upgrade to VS 2017

1760e83

Disable failing tests and add workarounds

2c015f2

We should be able to revert this entire commit when external issues are fixed

Fix test discovery failures on netcore

7f90062

Fix remaining failing tests

cbd95ba

natemcmaster force-pushed the namc/vs2017 branch from c2c4676 to cbd95ba Compare February 4, 2017 01:07

NTaylorMullen reviewed Feb 4, 2017

View reviewed changes

fixup! cleanup from feedback

f445965

Shadow copy false on .NET Framework tests

bc90f38

NTaylorMullen reviewed Feb 6, 2017

View reviewed changes

Suppress test log failures that don't actually indicate test failure

ea88ba8

Set IsPackable false on samples

a38509f

Nate McMaster added 2 commits February 6, 2017 16:52

XunitLogger swallows all exception types

4b79720

fixup! Take 13. Never-ending Linux tests

c897fd2

natemcmaster merged commit d367424 into dev Feb 7, 2017

natemcmaster deleted the namc/vs2017 branch February 7, 2017 02:09

		@@ -0,0 +1 @@
		wwwroot/lib/

Upgrade to VS 2017 #194

Upgrade to VS 2017 #194

Conversation

natemcmaster commented Feb 3, 2017 • edited Loading

dnfclas commented Feb 3, 2017

NTaylorMullen commented Feb 3, 2017

natemcmaster commented Feb 3, 2017

NTaylorMullen commented Feb 3, 2017

natemcmaster commented Feb 3, 2017

NTaylorMullen commented Feb 3, 2017

natemcmaster commented Feb 3, 2017

NTaylorMullen commented Feb 3, 2017

natemcmaster commented Feb 4, 2017 • edited Loading

NTaylorMullen commented Feb 4, 2017

natemcmaster commented Feb 4, 2017

natemcmaster commented Feb 4, 2017

NTaylorMullen commented Feb 4, 2017

natemcmaster commented Feb 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NTaylorMullen commented Feb 4, 2017

natemcmaster commented Feb 4, 2017 • edited Loading

natemcmaster commented Feb 4, 2017

Choose a reason for hiding this comment

natemcmaster commented Feb 6, 2017

natemcmaster commented Feb 6, 2017

NTaylorMullen commented Feb 6, 2017

natemcmaster commented Feb 3, 2017 •

edited

Loading

natemcmaster commented Feb 4, 2017 •

edited

Loading

natemcmaster commented Feb 4, 2017 •

edited

Loading