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.

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

Integration testing is hard to set up and may break on shared framework servicing #3156

Closed
dasMulli opened this issue May 17, 2018 · 23 comments

Comments

@dasMulli
Copy link
Contributor

dasMulli commented May 17, 2018

Sort of a follow-up to @poke's dotnet/sdk#2253 but orthogonal.

TL;DR: Unit tests should have an option to run on the asp.net core shared framework using the same servicing strategy that the main subject-under-test application uses, which means referencing the shared framework using the latest known version of the web SDK and applying NuGet and assembly resolution accordingly.

Story:
As a developer, I want to add a new test project with a reference to my asp.net core project so that I can write integration tests.

Naive steps:

  • dotnet new razor -o ../RazorPagesProject
  • dotnet new xunit
  • dotnet add reference ../RazorPagesProject
  • dotnet add package Microsoft.AspNetCore.Mvc.Testing -v 2.1.0-rc1-final
  • add C# code
    public class SampleIntegrationTests : IClassFixture<WebApplicationFactory<Startup>>
    {
        public HttpClient Client { get; }

        public SampleIntegrationTests(WebApplicationFactory<Startup> fixture)
        {
            Client = fixture.CreateDefaultClient();
        }

        [Fact]
        public async Task ItShallWork()
        {
            var response = await Client.GetAsync("/");
            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
        }
    }
  • dotnet test

Expected: Test passes

Actual:

Starting test execution, please wait...
[xUnit.net 00:00:00.5209160]   Discovering: SampleTests
[xUnit.net 00:00:00.5793000]   Discovered:  SampleTests
[xUnit.net 00:00:00.5854810]   Starting:    SampleTests
[xUnit.net 00:00:00.7291840]     SampleTests.SampleIntegrationTests.ItShallWork [FAIL]
[xUnit.net 00:00:00.7305700]       System.IO.FileNotFoundException : Could not load file or assembly 'Microsoft.AspNetCore, Version=2.1.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'. The system cannot find the file specified.
[xUnit.net 00:00:00.7306390]
[xUnit.net 00:00:00.7461290]   Finished:    SampleTests
Failed   SampleTests.SampleIntegrationTests.ItShallWork
Error Message:
 System.IO.FileNotFoundException : Could not load file or assembly 'Microsoft.AspNetCore, Version=2.1.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'. Thesystem cannot find the file specified.

Workaround:
Add a Package reference to the test project (!):

<ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.All" Version="2.1.0-rc1-final" />
</ItemGroup>

What I really want:

  • It should just work ™️
  • Integration tests shall run on the asp.net core shared framework
  • Integration tests shall use the same patch level of asp.net core components that dotnet run or dotnet publish + dotnet myapp.dll would use

What could be done here? a web-test-SDK maybe?

@dasMulli dasMulli changed the title Testing is hard to set up and may break on shared framework servicing Integration testing is hard to set up and may break on shared framework servicing May 17, 2018
@natemcmaster
Copy link
Contributor

We've discussed this quite a bit, and it's currently by-design. You need to add a PackageReference to your test projects that have a ProjectReference to web apps.

I agree it is less than ideal. We explored lots of options, but were unable to find a default that we felt comfortable with that would have fixed this without breaking other scenarios. The root of the problem is the default behavior of NuGet and ProjectReference. By default, NuGet will transitively flow compilation references but exclude the build and analyzer assets.

Some things that could be done: changing the default for PrivateAssets (NuGet/Home#6091), require manually setting PrivateAssets="None" on your PackageReference to Microsoft.AspNetCore.App, or use the Web SDK to update PrivateAssets. As we considered the impact of these, though, we didn't feel confident that these solved the problem any better. Some of these changes would have had other unintended consequences, so we decided to keep the current experience.

@dasMulli
Copy link
Contributor Author

dasMulli commented May 17, 2018

I see. was thinking about PrivateAssets before but I see that this could blow up easily as well..

Have there been discussions about adding a template for asp.net core integration tests? or "asp.net core app with integration test" multi-project templates

@poke
Copy link
Contributor

poke commented May 17, 2018

Doesn’t this completely defeat a major benefit of using the shared Microsoft.AspNetCore.App then? If I change the web project to have a versioned reference instead, e.g.:

<PackageReference Include="Microsoft.AspNetCore.App" Version="2.1.0-rc1-final" />

Then the test project is able to pull in the dependencies transitively and everything works.

To me that seems much better than having to make the test project a Web SDK project, or having to add an explicitly versioned dependency in just the test project that I need to update properly so it’s still in sync to the web application project.

@natemcmaster
Copy link
Contributor

If I change the web project to have a versioned reference instead, ... then the test project is able to pull in the dependencies transitively and everything works.

Actually, this is close, but still not quite right. You're getting the compilation references, but your not actually running on the aspnet shared framework. Your test project will run on the Microsoft.NETCore.App shared framework, not Microsoft.AspNetCore.App. (checkout the runtimeconfig.json file in your bin folder).

@natemcmaster
Copy link
Contributor

Have there been discussions about adding a template for asp.net core integration tests? or "asp.net core app with integration test" multi-project templates

It came up, but we haven't committed to it. We were unsure how much demand there is for this.

@poke
Copy link
Contributor

poke commented May 17, 2018

But would that be a problem? If not running on the AspNetCore shared framework makes everything easier to grasp, then maybe that’s a better experience for end users of the framework?

@natemcmaster
Copy link
Contributor

It could be. If you are running from the package cache, you don't get the same roll-forward and security-patch semantics as a shared framework would.

There are lots of problems here to consider. The shared framework aspect is one of them, but there are more. Nuspec generation, target framework compatibility, NuGet UI, minor version rollforward for shared frameworks, etc. I appreciate your feedback, and it's something we'll continue to consider. But I agree with @dasMulli - this is a problematic and tricky area in which there do not seem to be good options.

@dasMulli
Copy link
Contributor Author

Building a first-class

<TargetFramework>aspnetcoreapp2.1</TargetFramework>

could help but that has lots of other implications as well..

In the long run, this probably isn't going to get easier if a similar pattern is used for desktop apps on .net core 3.0.

@dasMulli
Copy link
Contributor Author

FWIW I've "documented" this on StackOverflow Q&A style https://stackoverflow.com/questions/50401152/integration-and-unit-tests-no-longer-work-on-asp-net-core-2-1-failing-to-find-as/50401153#50401153

@JunTaoLuo
Copy link
Contributor

FYI adding a template for tests is being tracked in https://github.com/aspnet/templating/issues/511 though it has not yet been triaged.

@poke
Copy link
Contributor

poke commented May 31, 2018

After working with this for a while now, there are two things that are really bothering me about having to make the test project a Web SDK project:

  • In Visual Studio, the test project has a normal web application icon; so I cannot quickly see that this is a test project.
  • This being a web project makes Visual Studio add a launchSettings.json to it by default, which is really unneded there. So one has to use <NoDefaultLaunchSettingsFile>True</NoDefaultLaunchSettingsFile> explicitly to disable that.

@DamianEdwards
Copy link
Member

We're going to look at adding a project template for this in 2.2.0. Thanks for the feedback folks.

@AdamDotNet
Copy link

I agree with @poke, the VS UI gets awkward (makes it look like a web project, adds connected services node, and adds launchsettings.json, I can "publish" my unit tests to Azure, etc.). Improved documentation and a template does helps people get it right though. It just would be nice if the VS UI wasn't misleading too.

@DamianEdwards Naive suggestion: why not have <Project Sdk="Microsoft.NET.Sdk.Test"> and <Project Sdk="Microsoft.NET.Sdk.Web.Test"> so that project nodes "definitively" look like test projects and work in their respective environments, netcoreapp testing and aspnetcoreapp testing? Additionally, the hypothetical test SDK could make <PackageReference Include="Microsoft.NET.Test.Sdk" Version="x.x.x" /> automagical, so users don't have to reference it from NuGet themselves. It would ideally just match the version of Visual Studio you are running.

@Eilon
Copy link
Member

Eilon commented Jul 16, 2018

@natemcmaster - regarding your earlier comments in this thread, does some of the new thinking around the runtime resolve the issue here?

(There's already an issue tracking the creation of a test project template: https://github.com/aspnet/Templating/issues/511)

@natemcmaster
Copy link
Contributor

It doesn't exactly resolve it. If we move forward with removing the PackageReference from the .csproj (#3307), users will still need to set the RuntimeFrameworkName property in each test project. Flowing the value of this property across ProjectReference is something we could investigate doing in 2.2 as a follow up to dotnet/sdk#2394, but that piece will be tricky to get right.

@natemcmaster
Copy link
Contributor

FYI - I've opened dotnet/sdk#2420 to track some SDK changes we need to address the issue of mismatch shared runtimes

@poke
Copy link
Contributor

poke commented Jul 20, 2018

@natemcmaster Just so I understand that new issue correctly: If that one is resolved, then test projects essentially only need to reference the web project and they don’t need to set up anything on their own anymore (e.g. Web SDK and an explicit AspNetCore.App reference)? If that’s the case, that would be perfect!

@natemcmaster
Copy link
Contributor

That's only one piece of the puzzle, but that change alone isn't enough to really resolve it. I expect a full solution will invoke both SDK changes, something along the lines of #3307, and probably creating some templates which demonstrate that its all working well.

@poke
Copy link
Contributor

poke commented Jul 20, 2018

Alright, thank you! I’m looking forward to seeing how this all turns out in the end.

@dasMulli
Copy link
Contributor Author

I believe this is now fixed by dotnet/sdk#2533 in 2.2 tooling 🎉

@dasMulli
Copy link
Contributor Author

TL;DR: Test projects can now have a versionless asp.net core reference that will match the referenced project (provided there isn't any conflict due to self-contained roll-forward).

@poke
Copy link
Contributor

poke commented Nov 14, 2018

Side note: It’s not perfectly resolved since testing the app should ideally pull in the needed references transitively, so a reference to the shared framework should not be necessary.

But if I understand it correctly, with 3.0’s framework references, the dependencies can be pulled transitively, so this should work out of the box again.

@dasMulli
Copy link
Contributor Author

let's see about framework reference. AFAIK it is now a NuGet but probably won't stay that way so we'll see, I'd suggest we open a new issue for that

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants