pact-workshop-dotnet-core-v1 icon indicating copy to clipboard operation
pact-workshop-dotnet-core-v1 copied to clipboard

Provider test could be improved slightly

Open mattfrear opened this issue 6 years ago • 7 comments

Hello

Firstly thanks for a great resource, it has been very helpful. I have a suggestion. When you run the provider tests, you suggest running the api using dotnet run and then in another command window, running the tests using dotnet test.

My suggestion is that the provider test could instead start the API under test using Webhost.CreateDefaultBuilder()

Then you wouldn't need to dotnet run, you could test it all using dotnet test. Example:

public ProviderApiTests(ITestOutputHelper output)
{
    _outputHelper = output;

    _pactServiceUri = "http://localhost:9001";
    _providerStateHost = WebHost.CreateDefaultBuilder()
        .UseUrls(_pactServiceUri)
        .UseStartup<TestStartup>()
        .Build();

    _providerStateHost.Start();

    _providerUri = "http://localhost:9000";
    _sut = WebHost.CreateDefaultBuilder()
        .UseUrls(_providerUri)
        .UseStartup<Startup>()
        .Build();

    _sut.Start();
}

I actually just wasted half a day trying to get it to run using the new ASP.NET Core 2.1 WebApplicationFactory<T>, ala https://docs.microsoft.com/en-us/aspnet/core/test/integration-tests?view=aspnetcore-2.1

The problem with using WebApplicationFactory is that when you create a client via var client = _factory.CreateClient(), it seems that only that client can access the TestServer - it's not accessible to other clients such as your local browser, or the PactVerifier. Therefore I had to use WebHost.CreateDefaultBuilder() as you have done.

mattfrear avatar Aug 21 '18 14:08 mattfrear

Hey, Matt thanks for raising this. Sorry about the delay getting back to you.

So I have been thinking about your suggestion. On the one hand, it makes running the tests more fluid as you say by reducing commands needed to run tests. But on the other, if this was a production test pack with lots of services would you want the tests to start services under test? I know some organisations do this and others that don't. I guess it depends on the organisation itself.

Also, I am worried about too much magic happening when people are learning something new. I don't know about you but when I am learning a new piece of tech I prefer to do most steps manually even if in a production environment I would automate them. It helps me understand the process of the technology I am learning about.

So on balance, I would prefer to keep the commands separated. Partly because I am unsure if in a production environment you would combine them but mostly not to confuse less experienced engineers who are learning about this tech.

Hopefully, that makes sense but thanks for taking the time to create a detailed issue I do appreciate it. Always good to get some feedback to think about!

tdshipley avatar Sep 05 '18 10:09 tdshipley

Yeah, there are quite a lot of moving parts to get all this working. My line of thinking was you're already starting the provider state webhost in the test constructor, so you might as well start the system under test too.

I think the most confusing bit is the provider state webhost and what it is for.

Thanks again for a good resource.

mattfrear avatar Sep 06 '18 08:09 mattfrear

No problem glad you found it useful! I will take a look at the provider state webhost bit and see if there is something I can do to make it clearer.

tdshipley avatar Sep 06 '18 08:09 tdshipley

All other examples I find around this topic do start the server in the test itself (https://github.com/pact-foundation/pact-net). From an automation standpoint, delegating the startup to something else (a script or even having it in a container) seems to be a lot of heavy lifting for a test right?

sergiusignacius avatar Jun 01 '19 09:06 sergiusignacius

Yeah it is and it could be improved. I’m a bit busy right now but I would like to iterate on this workshop a bit - it was written a year ago in a bit of a rush! PRs are gratefully received however ;)

tdshipley avatar Jun 01 '19 10:06 tdshipley

Ok! Wasn't sure you wanted people helping. I'm using the workshop to learn and write my own tests. I'll create a PR to try and update it!

On Sat, 1 Jun 2019, 11:32 Thomas Shipley, [email protected] wrote:

Yeah it is and it could be improved. I’m a bit busy right now but I would like to iterate on this workshop a bit - it was written a year ago in a bit of a rush! PRs are gratefully received however ;)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tdshipley/pact-workshop-dotnet-core-v1/issues/7?email_source=notifications&email_token=AADNNR4WXX7Y5GAX6NE2KXDPYJF45A5CNFSM4FQXVSU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWW5WAA#issuecomment-497933056, or mute the thread https://github.com/notifications/unsubscribe-auth/AADNNRYAOR6OVNXQ3EMLNPLPYJF45ANCNFSM4FQXVSUQ .

sergiusignacius avatar Jun 01 '19 13:06 sergiusignacius

Awesome well I hope it’s helpful and absolutely any contributions would be great!

tdshipley avatar Jun 01 '19 23:06 tdshipley