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

RFC: PactNet 5.0 PactVerifier Breaking Changes #457

Closed
adamrodger opened this issue May 5, 2023 · 0 comments · Fixed by #465
Closed

RFC: PactNet 5.0 PactVerifier Breaking Changes #457

adamrodger opened this issue May 5, 2023 · 0 comments · Fixed by #465
Assignees
Milestone

Comments

@adamrodger
Copy link
Contributor

Introduction

Below is a request for comments from the community about a potential breaking change to the Pact verifier in PactNet 5.0

Background

Pact specification v4 allows both synchronous HTTP pacts and asynchronous messaging pacts to be present in the same pact file, which was a key limitation of specification v3.

In PactNet 4.x we only support up to specification v3, and so the PactVerifier API enforces either verifying HTTP-style pacts or messaging-style pacts, but not both:

var verifier = new PactVerifier();

// HTTP style
verifier.ServiceProvider("My API", new Uri("http://localhost:5000"))
        .WithFileSource(...)
        .Verify();;

// Messaging style
verifier.MessagingProvider("My API")
        .WithProviderMessages(scenarios => { ... })
        .WithFileSource(...)
        .Verify();;

This means that any v4 pacts containing both HTTP and messaging interactions will fail to verify, because you can't specify both a HTTP and messaging provider on the verifier. Even though the v4 pacts themselves support shared pacts, PactNet 4.x does not.

Proposal

A breaking change is made to PactNet 5.x such that you can specify both a HTTP and messaging 'transport' (which is what the Pact FFI calls them internally):

var verifier = new PactVerifier("My API");

verifier
    .WithHttpEndpoint(new Uri("http://localhost:5000"))
    .WithMessages(scenarios =>
    {
        scenarios.Add<MyEvent>("an event happens")
    })
    .WithFileSource(new FileInfo(@"..."))
    .Verify();

The breaking changes are:

  • PactVerifier requires the provider name as a mandatory constructor parameter (otherwise it would have to be specified on both the HTTP and message transport, and they'd have to match)
  • ServiceProvider is renamed to WithHttpEndpoint and the provider name parameter is removed
  • MessagingProvider is renamed to WithMessages, the provider name parameter is removed and the messages are supplied directly instead of via an additional call to WithProviderMessages
    • This means IPactVerifierMessagingProvider is no longer needed and is deleted
  • All the WithXSource methods are moved to IPactVerifier
    • This means IPactVerifierProvider is no longer needed and is deleted

Caveats/Drawbacks

Both WithHttpEndpoint and WithMessages are optional in case a particular service doesn't have both types of interactions, but at least one must be provided and at most one of each type. I would ideally like these invariants to be enforced at compile time but that isn't really possible without introducing a very awkward type hierarchy. Consider:

  • After you add a HTTP transport you can either add a message transport also or move directly to specifying a source
    • If you add a message transport, you must move directly to adding a source
  • After you add a message transport you can either add a HTTP transport also or move directly to specifying a source
    • If you add a HTTP transport, you must move directly to adding a source

This means you have really awkward types like:

public interface IPactVerifier
{
    public IHttpAddedButNotMessageYet  WithHttpEndpoint();
    public IMessageAddedButNotHttpYet WithMessages();
}

public interface IMessageAddedButNotHttpYet : IPactVerifierTransportsConfigured
{
    // 👇 this is a duplicate of the method above in terms of signature, but with different return type
    public IPactVerifierTransportsConfigured WithHttpEndpoint();
}

public interface IHttpAddedButNotMessageYet : IPactVerifierTransportsConfigured
{
    // 👇 this is a duplicate of the method above in terms of signature, but with different return type
    public IPactVerifierTransportsConfigured WithMessages();
}

public interface IPactVerifierTransportsConfigured
{
    public IPactVerifierSource WithFileSource(...);
    public IPactVerifierSource WithDirectorySource(...);
    // etc
}

I don't think the added complexity to the types is worth the added runtime safety in this instance, particular given this is a testing library and we can simply throw InvalidOperationException if you try to add the same transport twice, or no transports at all. The feedback loop is fast, the error is easy to communicate/fix, and the code isn't production code.

It also creates a combinatorial explosion if in future we support more transport types.

@adamrodger adamrodger added the rfc label May 5, 2023
@adamrodger adamrodger added this to the 5.0.0 milestone Jun 27, 2023
@adamrodger adamrodger self-assigned this Jun 27, 2023
JohnWinston329 added a commit to JohnWinston329/pact-net that referenced this issue Dec 26, 2023
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

Successfully merging a pull request may close this issue.

1 participant