Coder Social home page Coder Social logo

Comments (30)

TimothyJones avatar TimothyJones commented on May 29, 2024 4

we still need to know and test same business logic.

I don't think the contract test should be testing business logic, though - it should be testing "can the client understand all the responses it might get".

I see the point about being able to write a test with multiple types in the array, though. However, we don't want to write tests that are like: "this array contains either A or B", which passes when the provider only ever returns A (this is the reason that eachLike has a minimum of 1 - so an empty array won't pass it during provider verification).

As above, I'd be in favour of a matcher that can express "this array contains an element like X anywhere in it".

from pact-specification.

uglyog avatar uglyog commented on May 29, 2024 2

I think the solution to this problem is to be able to apply matchers based on a predicate (See pact-foundation/pact-jvm#727)

For example:

validatedAnswers minLike(1) {
  when(type == ''typeA") {
    favourite_colour regex('blue|red')
    ...
  }
}

This should add a type matcher something like (see what I did there? ;-) )

    "matchingRules": {
          "body": {
            "$.validatedAnswers": {
              "matchers": [
                {
                  "match": "type",
                  "min": 1
                }
              ],
              "combine": "AND"
            },
            "$.validatedAnswers[*].favourite_colour": {
              "matchers": [
                {
                  "match": "regex",
                  "regex": "red|blue"
                }
              ],
              "combine": "AND",
              "predicate": [
                "$.validatedAnswers[@].type == 'typeA'" // this need to be related to the same array item somehow
              ]
            }
          }
        }

from pact-specification.

uglyog avatar uglyog commented on May 29, 2024 2

@zeldigas it is a work in progress, so no docs have been written to date. It is implemented in Rust and Pact-JS V3 and Pact C++. I'm currently implementing it for Pact-JVM. You can find an example in JS here: https://github.com/pactflow/example-siren/blob/master/consumer/src/__tests__/delete-order.spec.js#L73

from pact-specification.

mefellows avatar mefellows commented on May 29, 2024

You can still wrap the entire array in a like and I believe you can additionally restrict the value of event in history to approve and create using a regex. What you can't do at the moment is ask pact to validate that it contains exactly two elements within the array of n elements.

from pact-specification.

ivawzh avatar ivawzh commented on May 29, 2024

@mefellows If I use regex with a union value like:

expected_response = {
  history: Pact.like([
    {
      event: Pact.term(
        generate: 'approve', 
        matcher: /(approve|create)/)
      }),
      id: Pact.like(2)
    },
    {
      event: Pact.term(
        generate: 'create', 
        matcher: /(approve|create)/)
      }),
      id: Pact.like(1)
    }
  ]
})

It will fail for:

  • array length not matched
  • the second element in the actual response is neither approve or create.

For now, a dirty hack around would be:

expected_response = {
  history: Pact.like([
   {
      event: Pact.term(
        generate: 'create', 
        matcher: /(approve|create|health_check|verify)/)
      }),
      id: Pact.like(1)
    },
    {
      event: Pact.term(
        generate: 'Dont mind me. I am a placeholder for making length check pass.', 
        matcher: /(approve|create|health_check|verify)/)
      })
    },
    {
      event: Pact.term(
        generate: 'Dont mind me. I am a placeholder for making length check pass.', 
        matcher: /(approve|create|health_check|verify)/)
      })
    },
    {
      event: Pact.term(
        generate: 'approve', 
        matcher: /(approve|create|health_check|verify)/)
      }),
      id: Pact.like(1)
    }
  ]
})

Ugly because consumer has to

  1. know all the possible event names, though I only care two of them,
  2. fill with placeholders because of array length check.

Furthermore, it will give me false positive if the actual response contains 4 verify events. Also if provider changes to provide 7 events, test will be broken though consumer is not actually affected.

from pact-specification.

bethesque avatar bethesque commented on May 29, 2024

If you were to design the interface for this, what would you want it to look like, and what should it's pass/fail scenarios be?

from pact-specification.

ivawzh avatar ivawzh commented on May 29, 2024

For my current case, we only need something like

{
  history: Pact.array_contains(
    { event: 'create', id: Pact.like(1) }, 
    { event: 'create', id: Pact.like(1) }
  )
}

If I am greedy, I imagine it'd be good to also have

  • array_contains_either
  • array_contains_any
  • array_contains_none
  • aggregator:
  Pact.arrayThat(
    contains_either: [{ gender: 'male' }, { gender: 'female' }],
    contains_any: [{ origin: 'Australia' }, { origin: 'China' },  { origin: 'India' }]
    contains_none: [{ speak: 'Martian' }],
  )

But array_contains is really the only and urgent feature we need for now in many places.

from pact-specification.

bethesque avatar bethesque commented on May 29, 2024

I've been giving this some thought.

What should we do in the scenario where an element exists in an array that doesn't have an example specified for it?

expected = Pact.array_contains "red", "green"
actual = ["blue", "red", "green"]

Do we match blue based on type? Do we ignore it completely? It makes me a little nervous to be just ignoring elements, but I guess that's what the consumer is doing anyway.

With the contains_any, this asserting doesn't assert much. If all the elements come back as Australia, then we may still think that China and India are options when they may not be. I don't like the idea of making an assertion that is un-assertable. I guess it's the same as doing a regex Australia|China|India though, and that's already allowed. It just makes me uncomfortable. I wonder if we could have a warning for options that are never found.

One thing is that pact doesn't provide a "does not contain" assertion. This is for a few reasons. One, as each interaction contract only applies to one interaction, you can never be assured that a different request/response pair might not have the thing you are asserting doesn't exist. Secondly, just because you don't use a thing, doesn't mean another consumer doesn't want a thing. Being allowed to say "x has a thing" and "x does not have a thing" would lead to potential conflicts. Following Postel's law means a consumer should be relaxed about what it accepts, and ignore things that don't apply to it.

I understand this is a little different when we're talking about array elements rather than json nodes, however, I still feel it would be a bad feature choice.

Sorry, this is a lot of nit picking, but thinking through all the positive and negative implications of a feature is part and parcel with the specification design! It's gotta be done.

from pact-specification.

uglyog avatar uglyog commented on May 29, 2024

The issue is when people have lists with key value pairs, there can be any number but they are only interested in some of them. See pact-foundation/pact-jvm#378

from pact-specification.

ivawzh avatar ivawzh commented on May 29, 2024

I actually did not give too much thought before I mention array_contains_either, array_contains_any, array_contains_none. I agree with @bethesque, maybe in most of the common cases we should use context (given and upon_receiving) to help setup response instead of having array_contains_either, array_contains_any, array_contains_none to declare general response types. Some examples:

# example for array_contains
# the provider will stub some things to make sure at least one girl will be created. 
stubbed_service
  .given('I hope there will be at least one girl')
  .upon_receiving('make me 12 ponies')
  .will_respond_with(
    Pact.array_contains({ name: Pact.like('Bell'), gender: 'girl' })
   )

# example for array_contains_none
# the provider will stub some things to make sure all girls. 
stubbed_service
  .given('I hope there will be no boys')
  .upon_receiving('make me 12 ponies')
  .will_respond_with(
    Pact.each_like({ name: Pact.like('Bell'), gender: 'girl' })
   )

# example for array_contains_any
# the provider does not need to be stubbed. 
stubbed_service
  .given('boy or girl is up in the air')
  .upon_receiving('make me 12 ponies')
  .will_respond_with(
    Pact.each_like({ 
      name: Pact.like('Bell'), 
      gender: Pact.term(
        generate: 'girl', 
        matcher: /(boy|girl)/)
      })
    })
   )
)

I think in the array_contains_any example the consumer test will be a bit hacky because the generated array will only contain girl in this setup. Thus it cannot test if downstream logics can properly handle boy and girl at the same time. But it's a minor issue.

Contains_either is implying when one type occurs then the other type won't occur. That's effectively a 'does not contain' assertion. Hence we don't need to consider.

from pact-specification.

TimothyJones avatar TimothyJones commented on May 29, 2024

array_contains would have been useful on our last project - for modelling a set of returned elements, we don't necessarily know the order that they'll come in.

I agree with the arguments against array_contains_any and array_contains_none

from pact-specification.

mefellows avatar mefellows commented on May 29, 2024

from pact-specification.

bethesque avatar bethesque commented on May 29, 2024

I can see how this would be useful, but it also seems to me to be getting too much into the business logic of the response. Instead of worrying about the keys and types in the responses, we're starting to say, "when this key/value is like this, then the other key/value should be like this" and then we start putting business rules into the contracts that then make them quite brittle. I'm not against it, but I'm mulling through the possible consequences.

from pact-specification.

kiranpatel11 avatar kiranpatel11 commented on May 29, 2024

@bethesque not sure having predicates is against philosophy of contract testing.

I am working with below usecase :

` GET customer/12345/orders

Response
{
	Orders : [ 
	{
		orderId : "xyz",
		"status"   : "INPROGRESS",
		"orderCreatedOn" : "20-July-2019",
		…..
	
	},
	{
		orderId : "abc",
		"status"   : "COMPLETED",
		"orderCreatedOn" : "12-June-2019",
		"orderCompletedOn" : "20-June-2019",
		…..
	}]
}     `

The consumer expects the "orderCompletedOn" to be there when an order is in COMPLETED status. I think this is pretty much contractual in nature. In relatively complex business context, these kind of apis are norms.

from pact-specification.

TimothyJones avatar TimothyJones commented on May 29, 2024

The consumer expects the "orderCompletedOn" to be there when an order is in COMPLETED status. I think this is pretty much contractual in nature. In relatively complex business context, these kind of apis are norms.

I agree with this.

it also seems to me to be getting too much into the business logic of the response.

I also agree with this - I don't think it's the right approach to put logic into your pact test.

It sounds like you're looking for "the response to X is Y(a) if the provider state is S(a); but Y(b) if the provider state is S(b)".

Your contract will be more straightforward if you're able to say "when the provider state is S(a), and the request is X, the response is Y(a)" and "when the provider state is S(b), and the request is X, the response is Y(b)"

To do this I would use two tests, resulting in two separate interactions in the pact:

  1. With the provider state "an order is in progress"
  2. With the provider state "an order is completed"

from pact-specification.

kiranpatel11 avatar kiranpatel11 commented on May 29, 2024

With the provider state "an order is in progress"
With the provider state "an order is completed"

This is exactly what we have done, but was wondering if we have to write a testcase to simulate more realworld case of retrieving all customer orders. Semantically, I think it is still same thing even if we write tests with 2 different states, we still need to know and test same business logic.

from pact-specification.

mefellows avatar mefellows commented on May 29, 2024

The consumer expects the "orderCompletedOn" to be there when an order is in COMPLETED status. I think this is pretty much contractual in nature. In relatively complex business context, these kind of apis are norms.

I have this exact situation on a client now, where data coming in from a BMS system comes in as a list of records, with dynamic fields basic on the specific value of a key.

from pact-specification.

zeldigas avatar zeldigas commented on May 29, 2024

Any progress or ETA on that?

Looks like this feature is essential to create contracts on formats like Siren - it is list based, so to create a contract that entity has a link with specific rel and not put restriction on index in array, this feature described here is essential.

To be more specific, let's look at an example:

{
  "class": [ "order" ],
  "properties": { 
      "orderNumber": 42, 
      "itemCount": 3,
      "status": "pending"
  },
  "entities": [
    { 
      "class": [ "items", "collection" ], 
      "rel": [ "http://x.io/rels/order-items" ], 
      "href": "http://api.x.io/orders/42/items"
    },
    {
      "class": [ "info", "customer" ],
      "rel": [ "http://x.io/rels/customer" ], 
      "properties": { 
        "customerId": "pj123",
        "name": "Peter Joseph"
      },
      "links": [
        { "rel": [ "self" ], "href": "http://api.x.io/customers/pj123" }
      ]
    }
  ],
  "actions": [
    {
      "name": "add-item",
      "title": "Add Item",
      "method": "POST",
      "href": "http://api.x.io/orders/42/items",
      "type": "application/x-www-form-urlencoded",
      "fields": [
        { "name": "orderNumber", "type": "hidden", "value": "42" },
        { "name": "productCode", "type": "text" },
        { "name": "quantity", "type": "number" }
      ]
    }
  ],
  "links": [
    { "rel": [ "self" ], "href": "http://api.x.io/orders/42" },
    { "rel": [ "previous" ], "href": "http://api.x.io/orders/41" },
    { "rel": [ "next" ], "href": "http://api.x.io/orders/43" }
  ]
}

Right now Pact can only help with good verification of properties section. I have no idea how to verify presence of next rel and add-item action not relying on their positions in lists

from pact-specification.

TimothyJones avatar TimothyJones commented on May 29, 2024

I like this feature request, and a matcher for this example would be useful.

However, I think the example that @zeldigas has can be covered with provider states to dial down the state of the resource enough to be matched with the current matchers.

from pact-specification.

zeldigas avatar zeldigas commented on May 29, 2024

@TimothyJones could you advise how to do it with current set of matchers, taking into account that relying on position of item in list is not acceptable?
I'd be happy if you could educate me how to assert presence of previous link "item" in links, without pinning your assertion to current structure (i.e. that it's second item in a list). Client should not care about that, so the provider can put it in any position as well as supply 10+ links that client does not care about.

from pact-specification.

uglyog avatar uglyog commented on May 29, 2024

I have an initial prototype of the "Array Contains" matcher working in Rust.

Simple example: https://github.com/pact-foundation/pact-reference/blob/feat/v4-spec/rust/pact_matching/src/json.rs#L828
Siren example: https://github.com/pact-foundation/pact-reference/blob/feat/v4-spec/rust/pact_matching/src/json.rs#L882

from pact-specification.

uglyog avatar uglyog commented on May 29, 2024

Closing this issue as the matcher has been released for Pact-Rust and Pact-JS V3.

from pact-specification.

zeldigas avatar zeldigas commented on May 29, 2024

@uglyog I can't find description for syntax of this matcher in docs. Is it only in rust source code so far?

from pact-specification.

uglyog avatar uglyog commented on May 29, 2024

In that project, if you comment out the delete endpoint in the provider, and then run the Rust CLI verifier, you get this error:

$ ./target/debug/pact_verifier_cli -f '/home/ronald/Development/Projects/Pact/example-siren/consumer/pacts/Siren Order Provider-Siren Order Service.json' -p 8080

Verifying a pact between Siren Order Provider and Siren Order Service
  get root
    returns a response which
      has status code 200 (OK)
      includes headers
        "Content-Type" with value "application/vnd.siren+json" (OK)
      has a matching body (OK)
  get all orders
    returns a response which
      has status code 200 (OK)
      includes headers
        "Content-Type" with value "application/vnd.siren+json" (OK)
      has a matching body (FAILED)
  delete order
    returns a response which
      has status code 200 (FAILED)
      includes headers
      has a matching body (OK)


Failures:

1) Verifying a pact between Siren Order Provider and Siren Order Service - get all orders returns a response which 
    1.1) has a matching body
           $.entities.0.actions -> Variant at index 1 ({"href":"http://localhost:9000/orders/1234","method":"DELETE","name":"delete"}) was not found in the actual list

2) Verifying a pact between Siren Order Provider and Siren Order Service - delete order returns a response which 
    2.1) has status code 200
           expected 200 but was 405

There were 2 pact failures

from pact-specification.

zeldigas avatar zeldigas commented on May 29, 2024

Great, thank you so much! I'll give it a try

from pact-specification.

anurag-evive avatar anurag-evive commented on May 29, 2024

@uglyog Hey, was array_contains released for pact-jvm?

from pact-specification.

uglyog avatar uglyog commented on May 29, 2024

Yes, with version 4.2.0-beta

from pact-specification.

mefellows avatar mefellows commented on May 29, 2024

I couldn't find the updated matchers in the docs Ron, are they available in one of these pages: https://docs.pact.io/implementation_guides/jvm/consumer#building-json-bodies-with-pactdsljsonbody-dsl?

from pact-specification.

uglyog avatar uglyog commented on May 29, 2024

There are no secondary docs written yet. Primary docs are the source code :-D

docs.pact.io is updated from the master branch. The 4.2.x branch has not been merged to master yet (I'll probably do it this week)

Docs: https://github.com/pact-foundation/pact-jvm/tree/v4.2.x/consumer#array-contains-matcher-v4-specification

from pact-specification.

mefellows avatar mefellows commented on May 29, 2024

Ah! Of course, that makes sense.

from pact-specification.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.