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

MatchersV3.date and MatchersV3.time do not use example value on the provider side #1076

Open
4 of 5 tasks
agross opened this issue Mar 18, 2023 · 11 comments
Open
4 of 5 tasks
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@agross
Copy link

agross commented Mar 18, 2023

Software versions

  • OS: macOS 13.2.1
  • Consumer Pact library: Pact JS 11.0.2
  • Provider Pact library: PactNet 4.4.0
  • Node Version: v16.19.0

Issue Checklist

Please confirm the following:

  • I have upgraded to the latest
  • I have the read the FAQs in the Readme
  • I have triple checked, that there are no unhandled promises in my code and have read the section on intermittent test failures
  • I have set my log level to debug and attached a log file showing the complete request/response cycle
  • For bonus points and virtual high fives, I have created a reproduceable git repository (see below) to illustrate the problem

Expected behaviour

We use MatchersV3.date and MatchersV3.time. The docs state:

String value that must match the provided date/time format string. [...] If the example value is omitted, a value will be generated using a Date/Time generator and the current system date/time.

The expected behavior is that the provider will receive the passed example value of 10:11:12:

const x = MatchersV3.time("HH:mm:ss", "10:11:12")

Actual behaviour

The provider receives a generated date/time value, despite examples being given.

The Pact JSON file also contains generators for such date or time matchers.

Seems to be related to #620 which fixed it for MatchersV3.datetime.

While researching this I came across this document which seems to indicate that date/time values on the provider side could be dynamically generated. How can such expressions be included when generated values are used (i.e. no concrete examples are given, but a recipe describing the values to be generated). It would be very useful to use because "the current date/time" is not enough for most of our use-cases.

Steps to reproduce

const d = Matchers.date("yyyy-MM-dd", "2023-02-01");
expect(d["pact:generator:type"]).toBeUndefined();

const t = Matchers.time("HH:mm:ss", "10:11:12");
expect(t["pact:generator:type"]).toBeUndefined();
@agross agross added the bug Indicates an unexpected problem or unintended behavior label Mar 18, 2023
@TimothyJones
Copy link
Contributor

The expected behavior is that the provider will receive the passed example value of 10:11:12:

No, it isn't.

Matchers are for loosening the need for the provider to expect the exact same data used in the consumer tests. Have a look at the documentation on matchers

@agross
Copy link
Author

agross commented Mar 19, 2023

I'd say the purpose of MatchersV3 is twofold:

  • As you stated it can be used such that the provider does not need to return exact data in the response, but something that e.g. satisfies a type or regex, etc.
  • Some matchers will also include generator elements in the Pact JSON when given no example value. When using such Matchers in the request part of the Pact the Pact Verifier will then use the generator to craft a value while building the request to verify the provider.

This issue is about the second case. MatchersV3.datetime will not include the generator bits when being supplied with an example value, but MatchersV3.time and MatchersV3.date will.

I've created an example to illustrate the use-case. The important interaction is "A sample request with Matcher on request query string": it uses Matcher.time for the t query string, such that the stub server used by the frontend team accepts any time, not just 10:11:12:

$ http GET 'localhost:8080/test-query?t=10:11:12'
HTTP/1.1 200 OK
access-control-allow-origin: *
content-length: 33
content-type: application/json
date: Sun, 19 Mar 2023 08:11:10 GMT

{
    "answer": "42",
    "time": "10:11:12"
}

$ http GET 'localhost:8080/test-query?t=11:11:12'
HTTP/1.1 200 OK
access-control-allow-origin: *
content-length: 33
content-type: application/json
date: Sun, 19 Mar 2023 08:11:13 GMT

{
    "answer": "42",
    "time": "10:11:12"
}

The provider will be verified with a value of t that is generated by the Pact Verifier on the fly, because there is this generator piece in the Pact JSON:

{
  "consumer": { ... },
  "interactions": [
    {
      "description": "A request where Matcher is used on request",
      "request": {
        "generators": {
          "query": {
            "$.t[0]": {
              "format": "HH:mm:ss",
              "type": "Time"
            }
          }
        },
        "method": "GET",
        "path": "/test-query",
        "query": {
          "t": [
            "10:11:12"
          ]
        }
      },
...

By default that is the value of "now". While running the provider verification with debug logging:

verify_interaction{interaction="A request where Matcher is used on request"}: pact_matching: Applying query generators...
verify_interaction{interaction="A request where Matcher is used on request"}: pact_models::generators: Generator = Time(Some("HH:mm:ss"), None), Generated value = Ok("09:17:41")

See that the generated value is now (09:17:41) instead of the provided example value of 10:11:12. This is where MatchersV3.datetime differs: The provided example value would be passed instead of a generated one.

Example Pact:

import { pactOptions } from "@/pact/pact-options";
import { pactWith } from "jest-pact/dist/v3";
import { MatchersV3 } from "@pact-foundation/pact";
import "isomorphic-fetch";

global.setImmediate = jest.useFakeTimers as any;

pactWith(pactOptions, (interaction) => {
  interaction(
    "A sample request with exact response data",
    ({ provider, execute }) => {
      beforeEach(() => {
        return provider
          .uponReceiving("A request where exact data needs to be returned")
          .withRequest({
            method: "GET",
            path: "/test-exact",
          })
          .willRespondWith({
            status: 200,
            body: {
              answer: "42",
              time: "10:11:12",
            },
          });
      });

      execute("responds with exact data", async (mockserver) => {
        await fetch(mockserver.url + "/test-exact");
      });
    }
  );

  interaction(
    "A sample request with typed response data",
    ({ provider, execute }) => {
      beforeEach(() => {
        return provider
          .uponReceiving("A request where typed data needs to be returned")
          .withRequest({
            method: "GET",
            path: "/test-typed",
          })
          .willRespondWith({
            status: 200,
            body: {
              answer: MatchersV3.like("42"),
              time: MatchersV3.time("HH:mm:ss", "10:11:12"),
            },
          });
      });

      execute("responds with generated time", async (mockserver) => {
        await fetch(mockserver.url + "/test-typed");
      });
    }
  );

  interaction(
    "A sample request with Matcher on request query string",
    ({ provider, execute }) => {
      beforeEach(() => {
        return provider
          .uponReceiving("A request where Matcher is used on request")
          .withRequest({
            method: "GET",
            path: "/test-query",
            query: {
              t: MatchersV3.time("HH:mm:ss", "10:11:12"),
            },
          })
          .willRespondWith({
            status: 200,
            body: {
              answer: MatchersV3.like("42"),
              time: "10:11:12",
            },
          });
      });

      execute("responds with time from request", async (mockserver) => {
        await fetch(mockserver.url + "/test-query?t=10:11:12");
      });
    }
  );
});

@agross
Copy link
Author

agross commented Mar 19, 2023

Interesting, this bit in the the JSON will compute now + 2 hours for provider verification:

"generators": {
  "query": {
    "$.t[0]": {
      "format": "HH:mm:ss",
      "expression": "+ 2 hours",
      "type": "Time"
    }
  }
}

@TimothyJones
Copy link
Contributor

I think there’s a misunderstanding here- either yours or mine.

As I understand it, one way you can think of matchers is “this test covers every case that passes this matcher”. It’s not a description of the API- if you need an exact match, you just use the exact value.

I’m not completely across generators, as they’re after my time as a pact maintainer - but my understanding is that they’re just a way of building default values for matchers.

Following this, it would be valid for a pact implementation to use the generator OR the example during verification. If you need an exact value during verification, it should be written into the pact as an exact value.

@agross
Copy link
Author

agross commented Mar 19, 2023

The problem especially with date and time values is that sometimes the domain requires them to be in the future, but maybe also not too much in the distant future. In these cases, static values won't cut it.

Consider a hotel reservation system where reservations might be acceptable from today until today + 1 year, but not beyond. You could use generators for this, as the docs for date/time expressions state:

These expressions work relative to a base date + time, normally the current system clock. They provide a reliable way to work with relative dates in tests.

The consumer could specify a reservation request for from tomorrow until tomorrow + 2 days should be acceptable. The provider can then be verified against values generated from the current (real) system date and time + 1 day and +3 days without having to fiddle with mocking the system clock. This is how I understand generators. Very valuable for provider verification.

Perhaps the naming of the API is a bit misleading. Matchers imply what you and I understand, stated as bullet point 1 above. Matching is useful if returned data (e.g. UUIDs) cannot be known at the time when writing consumer tests because the data is dynamic.

But... It appears that the Matchers API can also be used to define generators in the Pact JSON (bullet point 2). Perhaps another API would be useful to differentiate between matching values for verification and generating values to issue requests.

On the other hand, I can see why both concepts are included in a single API. As far as I understand it generating values is also useful when you run the stub server, where maybe you want to return stubbed responses including timestamps that always reflect a future date.

@TimothyJones
Copy link
Contributor

TimothyJones commented Mar 19, 2023

I think normally you would handle that case with provider state- ie, “the time is {some specific time}”

@agross
Copy link
Author

agross commented Mar 19, 2023

Docs on generators do exist:

[Generating contents] will happen in consumer tests when the mock server needs to generate a response or the contents of a message, or during verification of the provider when the verifier needs to send a request to the provider.

Despite all of the above, this issue exists because MatchersV3.datetime and MatchersV3.date/MatchersV3.time handle example values differently.

No matchers appear to handle expressions for date and time generation currently.

@seyfer
Copy link

seyfer commented Mar 19, 2023

Looks like @agross has the right point here.

If other Matchers do not use generators when an example value is provided, then MatchersV3.datetime and MatchersV3.date/MatchersV3.time should behave in the same way for constancy.

Otherwise, while they do behave differently at the moment, it is confusing and causes issues.

@TimothyJones
Copy link
Contributor

while they do behave differently at the moment, it is confusing and causes issues.

Yes, I agree that it would be better to be consistent - even if the behaviour is an implementation detail rather than an expectation, it would be an improvement to keep that implementation detail consistent across all matchers.

I also agree that there's a naming issue with matchers in general - they're more like EquivalentTo or something, meaning "this test covers all cases that pass this matcher", not "here's a description of the API".

My point was that I don't think it's expected behaviour that the provider verification will use the example (although it might). I think appropriate uses of matchers would mean that the test would be valid with any example that passes the matcher. If you need a specific, exact value, then that specific value should be used instead of a matcher.

If it is the case that there needs to be some constraints on the value (like the example of "within $X time from now") or it depends on some other field in the request, then I think that is best addressed with provider state variables.

@mefellows
Copy link
Member

Despite all of the above, this issue exists because MatchersV3.datetime and MatchersV3.date/MatchersV3.time handle example values differently.

You're right, the definitions are slightly different:

export function datetime(format: string, example: string): DateTimeMatcher {
  if (!example) {
    throw new Error(`you must provide an example datetime`);
  }

  return pickBy((v) => !isNil(v), {
    'pact:generator:type': example ? undefined : 'DateTime',
    'pact:matcher:type': 'timestamp',
    format,
    value: example,
  });
}

vs

/**
 * String value that must match the provided date format string.
 * @param format Date format string. See [Java SimpleDateFormat](https://docs.oracle.com/javase/8/docs/api/java/text/SimpleDateFormat.html)
 * @param example Example value to use. If omitted a value using the current system date will be generated.
 */
export function date(format: string, example: string): DateTimeMatcher {
  if (!example) {
    throw new Error(`you must provide an example date`);
  }
  return {
    format,
    'pact:generator:type': 'Date',
    'pact:matcher:type': 'date',
    value: example,
  };
}

The datetime will never produce a generator, because example can't be falsey.

My thinking here is that you should be opting in to generators explicitly, and that by having generators mixed into a matcher makes the behaviour confusing and unexpected.

I could see a few ways to do it:

  1. Make example optional, and if not provided, it will generate a value on the fly (this is slightly more intuitive at least)
  2. Create a new XYZGenerated version (more explicit, but pollutes the API a bit)
  3. Support the expression language linked above (I'm not actually sure that was/is intended for use outside of plugins, but I could see the use case "now + 2 hours" type thing)

@uglyog is the expression language linked above designed for use outside of plugins?

@rholshausen
Copy link

These predate plugins, and are supported with V3 generators

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

5 participants