DiUS / pact-workshop-dotnet-core-v3

Apache License 2.0
24 stars 24 forks source link

refactor: update to official packages, use aspnet minimal api, net60 #7

Open riezebosch opened 1 year ago

riezebosch commented 1 year ago

Fixes #6

riezebosch commented 1 year ago

~while updating the readme I figure out that there is a (possible) mismatch in the authorization exercises now I changed to minimal api.~

should be fine but did not do a dry-run.

mefellows commented 1 year ago

This is great, thank you! Let's see if we can get some qualified eyeballs on this

YOU54F commented 1 year ago

HI @riezebosch

I had a review of this last night, thanks for this.

Some notes

  1. Code snippets in README are slightly out of date BaseUrl vs _baseUri
  2. The provider state middleware (including state handlers) has been removed, and there is instructions to the user on how to set up the middleware
  3. Provider returns a 200 instead of a 404 where there are no products, this is a change from the previous provider codebase (it highlights that Pact would have caught this change, on the provider side, if the provider rewrote their application) 👍🏾
    • provider test
  4. Pact-Net beta used to error with missing provider states, the current version 4.5.0 just warns. This means the user sees one failing test, which is because there are no state handlers enabled and therefore nothing to set the provider state "product with ID 11 does not exist"

All in all, solid effort though!

Once I went through the workshop, I was able to apply the delta against the current master and get that working, it was quite small

I also took the time to fix up

Regarding the outstanding issues raised

I've applied #2 , #3, #8, #10

1 is fixed by using PactNet and PactNet.Output.Xunit as this provides logging from the rust core, output to console ( I don't believe logging to file is supported in pact-net yet)

5 occurred due to pact-net-beta failing if there was an existing pact, it wasn't set to overwrite, so needed deleting and the tests rerunning

Could I ask, what is?

Do you have an example of hooking this up with the auth and provider state middleware?

riezebosch commented 1 year ago

Sorry, I wasn't aware of your review already. Just pushed some minor changes in standard C# naming conventions. But will make sure I will look into your comments.

YOU54F commented 1 year ago

No rush at all chap!

riezebosch commented 1 year ago

Tutorial: Create a minimal API with ASP.NET Core.

So minimal api reduces the boilerblate and focuses more on the REST API endpoints. In normal scaffolding (and also that tutorial) everything is just places in the Program.cs which makes it less ideal to be used from the (integration) tests. That's why I refactored that into the Startup class. Major difference with the old style startup class is that it's a just a POCO without the magic lifecycle methods that were used before.

So what I tried to do: update to minimal API to create a simpler example, but refactor it into something that opens up for testing (something that Microsoft and its examples tend to forget).

YOU54F commented 1 year ago

Yeah that makes total sense, it was easy enough to update just the test code, making for a smaller delta.

I think there is value in showing an alternative API style, but assume that there would be a need for both styles.

Looking at the examples, I think it should be easy enough to support both side by side, and gets me into the idea of having a canonical example and then having sub-workshops which can show the same thing but with a different framework style for the API, or a different testing framework for example (thinking wider than just .NET, but for all the workshops)