[Fixes #6233] Productionize and harden our functional testing infrastructure #6433

javiercn · 2017-06-22T18:25:15Z

Adds APIs for bootstrapping an application in memory.
Integration with our current functional test infrastructure is pending.
Open questions:

Should we plug-in the cookie container handler by default?

rynowak · 2017-06-22T21:14:18Z

src/Microsoft.AspNetCore.Mvc.Testing/CookieContainerHandler.cs

@@ -0,0 +1,38 @@
+using System.Net;


license header

rynowak · 2017-06-22T21:16:29Z

src/Microsoft.AspNetCore.Mvc.Testing/Microsoft.AspNetCore.Mvc.Testing.csproj

+  <ItemGroup>
+    <PackageReference Include="Microsoft.AspNetCore.TestHost" Version="$(AspNetCoreVersion)" />
+    <PackageReference Include="Microsoft.AspNetCore.Testing" Version="$(AspNetCoreVersion)" />
+    <PackageReference Include="xunit" Version="$(XunitVersion)" />


Can we drop Microsoft.AspNetCore.Testing and xunit from here? I know we will get feedback about bundling xunit with this.

I started with the bundled approach as its what we talked about for simplicity. We can definitely make it two packages
Microsoft.AspNetCore.Mvc.Testing and Microsoft.AspNetCore.Mvc.Testing.Xunit

I'm not convinced that there's any need for an xunit specific package. The most valuable piece of this is the test fixture and that's not xunit specific.

rynowak · 2017-06-22T21:17:14Z

src/Microsoft.AspNetCore.Mvc.Testing/Microsoft.AspNetCore.Mvc.Testing.csproj

+
+  <ItemGroup>
+    <Content Include="build\**\*.targets" Pack="true" PackagePath="%(Identity)" />
+    <Content Include="build\xunit.runner.json" Pack="true" PackagePath="%(Identity)" />


I'm not sure we should be overriding users xunit.runner.json file.

This only includes it in the package. That said, what we can do is look in the output folder if an xunit.runner.json file exists and only if it doesn't, we can copy ours to the output.

rynowak · 2017-06-22T21:18:20Z

src/Microsoft.AspNetCore.Mvc.Testing/MvcWebApplicationBuilder.cs

+            }
+            while (directoryInfo.Parent != null);
+
+            throw new Exception($"Solution root could not be located using application root {applicationBasePath}.");


This is really nice

rynowak · 2017-06-22T21:18:57Z

src/Microsoft.AspNetCore.Mvc.Testing/MvcWebApplicationBuilder.cs

+
+                builder.UseStartup<TestStartup<TStartup>>();
+
+                return new TestServer(builder);


It would be nice to have these verify that the .deps file is present (and anything else required). This is the most common issue that our users run into.

We could add checks based on whether the user called UseApplicationAssemblies and maybe we can do something to verify the xunit configuration.

There are a few ways of configuring XUnit, not only xunit.runner.json so we can't really tell for sure if shadowCopy (what we care about) is disabled.

Are you ok if we copy xunit.runner.json by default (if the file does not exist on the output) and we provide a property on MSBuild to disable it? We can add a validation in code that checks for xunit.runner.json on the current directory and prints to the console in case its not there or doesn't contain the values that we need. (We can disable the logging to with an environment variable, for the case you go "i know what i'm doing" all the way)

I would really prefer that this package doesn't have anything related xunit anything in it. I think it's just not needed, and if we stick xunit stuff in here users of other testing frameworks going to want it taken out.

Follow my reasoning for a set.

We have an msbuild targets file that copies the deps file

Make it an msbuild error if your 'app' project doesn't have a deps file

Then at runtime if you don't have a deps file the only possibility is that you're shadow copied. This should throw an exception and tell you to turn off shadow copying.

rynowak · 2017-06-22T21:20:34Z

src/Microsoft.AspNetCore.Mvc.Testing/CultureReplacerMiddleware.cs

+    /// A middleware that ensures web sites run in a consistent culture. Currently useful for tests that format dates,
+    /// times, or numbers. Will be more useful when we have localized resources.
+    /// </summary>
+    public class CultureReplacerMiddleware


It would be nice to pull a bunch of this stuff out of the Mvc.Testing namespace into something more obscure. You really only need a few of these classes.

yeah, pretty much everything but WebApplicationBuilder can be on the .Internal namespace

rynowak · 2017-06-22T21:22:04Z

src/Microsoft.AspNetCore.Mvc.Testing/WebApplicationTestFixture.cs

+    /// XUnit fixture for bootstrapping an application in memory for functional end to end tests.
+    /// </summary>
+    /// <typeparam name="TStartup">The applications startup class.</typeparam>
+    public class WebApplicationTestFixture<TStartup> : IDisposable where TStartup : class


It's nice that we can ship this without any direct xunit dependency.

rynowak · 2017-06-22T21:22:24Z

src/Microsoft.AspNetCore.Mvc.Testing/build/Microsoft.AspNetCore.Mvc.Testing.targets

@@ -0,0 +1,41 @@
+<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">


This is great

Could we use this in our functional test? So we test the tester?

We do but through a direct import, due to the fact that target files are only automatically imported for package references, we'll have some E2E tests for this on the identity repo.

rynowak · 2017-06-22T21:54:28Z

src/Microsoft.AspNetCore.Mvc.Testing/build/Microsoft.AspNetCore.Mvc.Testing.targets

+      assembly dependencies in websites to their corresponding bin/{config}/refs folder we need to re-calculate
+      reference assemblies for this project so there's a corresponding refs folder in our output. Without it
+      our websites deps files will fail to find their assembly referenes.
+    -->


Is this just about our stuff?

I'm not sure about that, I just copied all the custom stuff from the functional test project into the targets file. @pranavkm any idea what this comment is for?

rynowak

This looks like a great start. Feel free to do an initial check in and then do follow ups for issues, or resolve stuff now if you'd prefer that.

pranavkm · 2017-06-23T17:41:08Z

test/Microsoft.AspNetCore.Mvc.FunctionalTests/MvcEncodedTestFixtureOfT.cs

-                services.AddTransient<HtmlEncoder, HtmlTestEncoder>();
-                services.AddTransient<JavaScriptEncoder, JavaScriptTestEncoder>();
-                services.AddTransient<UrlEncoder, UrlTestEncoder>();
+                services.TryAddTransient<HtmlEncoder, HtmlTestEncoder>();


Wouldn't we want to force add these (and not TryAdd)?

That's why they are added before startup configure services run. It tests that even if you do it this way it will work.

…ructure

javiercn requested review from Eilon and rynowak June 22, 2017 18:25

dnfclas added the cla-already-signed label Jun 22, 2017

rynowak reviewed Jun 22, 2017

View reviewed changes

src/Microsoft.AspNetCore.Mvc.Testing/CookieContainerHandler.cs Outdated

@@ -0,0 +1,38 @@

using System.Net;

Copy link

Member

rynowak Jun 22, 2017

Choose a reason for hiding this comment

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

license header

rynowak reviewed Jun 22, 2017

View reviewed changes

pranavkm reviewed Jun 23, 2017

View reviewed changes

javiercn added 5 commits June 23, 2017 12:51

[Fixes #6233] Productionize and harden our functional testing infrast…

cea96eb

…ructure

Namespace clenaup

616bd0d

Integration with MVC functional test

51a518a

Fixed all MVC tests

d1990f9

Fix build

6ef0431

rynowak mentioned this pull request Jun 23, 2017

HTTP 404 with ASP.NET Core Test Server when controllers are added to a seperate assembly #5992

Closed

Addressed feedback

5b6d73e

javiercn force-pushed the javiercn/6233 branch from 9ff2fc4 to 5b6d73e Compare June 23, 2017 23:22

javiercn merged commit 5b6d73e into dev Jun 24, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fixes #6233] Productionize and harden our functional testing infrastructure #6433

[Fixes #6233] Productionize and harden our functional testing infrastructure #6433

javiercn commented Jun 22, 2017

rynowak Jun 22, 2017

rynowak Jun 22, 2017

javiercn Jun 23, 2017

rynowak Jun 23, 2017

rynowak Jun 22, 2017

javiercn Jun 23, 2017

rynowak Jun 22, 2017

rynowak Jun 22, 2017

javiercn Jun 23, 2017

rynowak Jun 23, 2017

rynowak Jun 22, 2017

javiercn Jun 23, 2017

rynowak Jun 22, 2017

rynowak Jun 22, 2017

pranavkm Jun 22, 2017

javiercn Jun 23, 2017

rynowak Jun 22, 2017

javiercn Jun 23, 2017

rynowak left a comment

pranavkm Jun 23, 2017

javiercn Jun 23, 2017


		builder.UseStartup<TestStartup<TStartup>>();

		return new TestServer(builder);

		@@ -0,0 +1,41 @@
		<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

[Fixes #6233] Productionize and harden our functional testing infrastructure #6433

[Fixes #6233] Productionize and harden our functional testing infrastructure #6433

Conversation

javiercn commented Jun 22, 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

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

rynowak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment