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

Provider test could be improved slightly #7

Open
mattfrear opened this issue Aug 21, 2018 · 7 comments
Open

Provider test could be improved slightly #7

mattfrear opened this issue Aug 21, 2018 · 7 comments

Comments

@mattfrear
Copy link

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, 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 mattfrear changed the title Provider test could be made better Provider test could be improved slightly Aug 21, 2018
@tdshipley
Copy link
Collaborator

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!

@mattfrear
Copy link
Author

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.

@tdshipley
Copy link
Collaborator

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 tdshipley reopened this Sep 6, 2018
@sergiusignacius
Copy link

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?

@tdshipley
Copy link
Collaborator

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 ;)

@sergiusignacius
Copy link

sergiusignacius commented Jun 1, 2019 via email

@tdshipley
Copy link
Collaborator

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

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

No branches or pull requests

3 participants