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

Problems with Mock Server when running Pact with Jest #1060

Closed
5 tasks done
subodhgodbole opened this issue Feb 14, 2023 · 16 comments
Closed
5 tasks done

Problems with Mock Server when running Pact with Jest #1060

subodhgodbole opened this issue Feb 14, 2023 · 16 comments

Comments

@subodhgodbole
Copy link

subodhgodbole commented Feb 14, 2023

Software versions

  • OS: Windows 10
  • Consumer Pact library: Pact JS v10.4.1.
  • Provider Pact library: Pact JS v10.4.1.
  • Node Version: 14.20.1

Issue Checklist

  • 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 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 Behavior

Mock server should start and respond as per Interactions defined.

Actual Behavior and Steps to reproduce

I am trying to use Pact in my Angular 13 workspace with Jest for writing contract tests.
I am using latest version of Pact which is v10.4.1.

However, I am running into problems related to Mock Server.
It seems that Mock Server is not receiving any requests.
I have added multiple debug logs to check which URL is used by Angular's HttpClient and it appears to point correctly to Mock Server's dynamic URL. See this -

console.log
**** Adding Interaction with path: /users/1
  at src/app/services/user.service.pact.spec.ts:44:15

console.log
**** MockServer:: URL: http://127.0.0.1:50118, ID: unknown
  at src/app/services/user.service.pact.spec.ts:65:17

console.log
**** UserService.get(): http://127.0.0.1:50118/users/1
  at UserService.get (src/app/services/user.service.ts:29:13)

From above -

  • Mock Server is running at http://127.0.0.1:50118.
  • Has one interaction registered with path as /users/1.
  • And Client is making http request to http://127.0.0.1:50118/users/1.

But still it's not working.

Also, I am not sure why Mock Server Id is coming out as "undefined".

Error I get is as below -

RUNS  src/app/services/user.service.pact.spec.ts
2023-02-08T10:33:00.360413Z DEBUG ThreadId(01) pact_matching::metrics: Could not get the tokio runtime, will not send metrics - there is no reactor running, must be called from the context of a Tokio 1.x runtime
2023-02-08T10:33:00.360795Z DEBUG ThreadId(01) pact_mock_server::server_manager: Shutting down mock server with ID ca85dcf4-01b7-4d4e-af7a-890baaa75559 - MockServerMetrics { requests: 0 }
2023-02-08T10:33:00.363789Z DEBUG ThreadId(01) pact_mock_server::mock_server: Mock server ca85dcf4-01b7-4d4e-af7a-890baaa75559 shutdown - MockServerMetrics { request  console.error
    Unhandled Promise rejection: Test failed for the following reasons:

      Mock server failed with the following mismatches:

        0) The following request was expected but not received:
            Method: GET
            Path: /users/1 ; Zone: ProxyZone ; Task: Promise.then ; Value: Error: Test failed for the following reasons:

      Mock server failed with the following mismatches:

        0) The following request was expected but not received:
            Method: GET
            Path: /users/1
        at PactV3.<anonymous> (C:\angular-pact\node_modules\@pact-foundation\src\v3\pact.ts:227:29)
        at step (C:\angular-pact\node_modules\@pact-foundation\pact\src\v3\pact.js:33:23)
        at Object.next (C:\angular-pact\node_modules\@pact-foundation\pact\src\v3\pact.js:14:53)
        at fulfilled (C:\angular-pact\node_modules\@pact-foundation\pact\src\v3\pact.js:5:58)
        at _ZoneDelegate.Object.<anonymous>._ZoneDelegate.invoke (C:\angular-pact\node_modules\zone.js\bundles\zone-testing-bundle.umd.js:409:30)

May be I am missing something very trivial/basic, I would really appreciate if you have any clue / suggestion on what might be wrong in my project?

I have uploaded my Angular project to GitHub here, where this is reproducible. (After cloning, command to run is npm run test:pact).

PS: I raised this on stackoverflow few days back. Also raising it here, in anticipation to get resolution faster.

@subodhgodbole subodhgodbole added the bug Indicates an unexpected problem or unintended behavior label Feb 14, 2023
@mefellows mefellows added Triage and removed bug Indicates an unexpected problem or unintended behavior labels Feb 14, 2023
@mefellows
Copy link
Member

Thanks for the detailed report and repro - much appreciated! I'm going to remove the bug label until we can verify it as such.

       Path: /users/1 ; Zone: ProxyZone ; Task: Promise.then ; Value: Error: Test failed for the following reasons:

This makes me think there is a proxy of sorts intercepting requests - probably as part of the Angular setup.

I'm not an Angular developer, but I know it does funky things with the HTTP clients in the test bed (or whatever they call it), so this is where I'd be investigating. Alternatively, there is some promise issue where the promises are firing out of order.

@TimothyJones
Copy link
Contributor

Can you share the test that generates this problem?

@mefellows
Copy link
Member

I think it's this one Tim: https://github.com/subodhgodbole/angular-pact/blob/main/src/app/services/user.service.pact.spec.ts

My guess is that it's something to do with Angular proxying/interfering with HTTP requests. It could probably be shown to be the issue by using a different HTTP client (e.g. axios). If it works, you are likely to believe it's something to do with the Angular setup. If it doesn't work, then it would be a bit more digging, but one would also have to suspect some other deeper dependency/configuration is at work (because we have many working examples)

@TimothyJones
Copy link
Contributor

That test isn't right. It's not handling the promise from executeTest so it probably succeeds even when it fails.

    it('should get a User', (done) => {
      provider.executeTest(async(mockServer) => {
        console.log(`**** MockServer:: URL: ${mockServer.url}, ID: ${mockServer.id}`);
        service.setUrlPrefix(mockServer.url);

        service.get(userId).subscribe({
          next: (response) => {
            console.debug(response);
            expect(response).toEqual(expectedUser);
            done();
          },
          error: (error: HttpErrorResponse)=> {
            done();
          }
        });
      });
    });

should be (at least):

    it('should get a User', () => {
      return provider.executeTest(async (mockServer) => {
        console.log(
          `**** MockServer:: URL: ${mockServer.url}, ID: ${mockServer.id}`
        );
        service.setUrlPrefix(mockServer.url);

        service.get(userId).subscribe({
          next: (response) => {
            console.debug(response);
            expect(response).toEqual(expectedUser);
          },
          error: (error: HttpErrorResponse) => {
            throw error;
          },
        });
      });
    });

@TimothyJones
Copy link
Contributor

Yep. This one is entirely unhandled promises at fault - the problem is that the Observable thing in angular doesn't wait for the request to be sent, so the executeTest callback returns immediately, and Pact thinks that the request has been sent before it has.

I don't know what you're supposed to do idomatically in Angular. But, this updated test works:

    it('should get a User', () => {
      return provider.executeTest(async (mockServer) => {
        console.log(
          `**** MockServer:: URL: ${mockServer.url}, ID: ${mockServer.id}`
        );
        service.setUrlPrefix(mockServer.url);

        return new Promise<void>((resolve, reject) => {
          service.get(userId).subscribe({
            next: (response) => {
              console.debug(response);
              expect(response).toEqual(expectedUser);
              resolve();
            },
            error: (error: HttpErrorResponse) => {
              reject(error);
            },
          });
        });
      });
    });

I have triple checked, that there are no unhandled promises in my code and have read the section on intermittent test failures

^ This means that you need to make sure that the executeTest doesn't complete before you have sent your request. This should probably be highlighted clearer in the documentation.

@subodhgodbole
Copy link
Author

subodhgodbole commented Feb 15, 2023

Thanks @TimothyJones. You pointed it right, test is finishing early and not waiting for executeTest() to finish. IMO, it's probably the Jest and not Angular who is behaving like this here.

Exactly for this async behavior Jest offers done callback for tests. As per Jest documentation, it should wait till done() is called. And if you have noticed, I had used done() and called it only when executeTest() completed. But it did not work. :-(

    it('should get a User', (done) => {
      provider.executeTest(async(mockServer) => {
        console.log(`**** MockServer:: URL: ${mockServer.url}, ID: ${mockServer.id}`);
        service.setUrlPrefix(mockServer.url);

        service.get(userId).subscribe({
          next: (response) => {
            console.debug(response);
            expect(response).toEqual(expectedUser);
            done();
          },
          error: (error: HttpErrorResponse)=> {
            done();
          }
        });
      });
    });

I have used this done() callback at other places as well like beforeAll(), afterEach() and it worked there.

@TimothyJones
Copy link
Contributor

TimothyJones commented Feb 15, 2023

No, that is not right. In that example,done is not correctly used.

Here is the sequence of events:

  1. Set up pact expectation
  2. Send request
  3. Receive response
  4. Verify that the provider received the correct information

In your example, you have written this:

  1. Set up pact expectation
  2. Send request
  3. Receive response
  4. Call done
  5. Verify that the provider received the correct information (this is now ignored by Jest)

In general, it's better to use the promise form instead of the callback form, as it avoids these kind of issues (I think using done is actually a lint error in the recommended jest eslint rules).

@subodhgodbole
Copy link
Author

subodhgodbole commented Feb 15, 2023

Nope! That does not appear to be the case as per code.
In code I am calling done() in next after service returns data. So theoretically it appears right.
Mock Server has work to do in between service.get(userId) and .subscribe().
And I am asserting tests in next and calling done() at the end.

@TimothyJones
Copy link
Contributor

You are not using done correctly. See my earlier messages, and try the corrected code I gave you.

@subodhgodbole
Copy link
Author

subodhgodbole commented Feb 15, 2023

I have pushed one more commit to my Git Repo. In this commit -

  • I have modified Pact test case - user.service.pact.spec.ts
    Used Promise approach and not done callback (as per code you provided).
    Now, this runs as expected, thanks very much for solving core problem here.
    Command to run is - npm run test:pact
 RUNS  src/app/services/user.service.pact.spec.ts
 PASS  src/app/services/user.service.pact.spec.ts (20.505 s)pact: Note: Existing pact is an older specification version (V2), and will be up  UserServicePact
    get()
      √ should get a User (192 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        39.183 s
  • Also added standard test case for Jest without Pact - user.service.jest.spec.ts
    Here I am using done callback, exactly the same way I was using it in Pact test earlier.
    This also gets passed correctly.
    Command to run is - npm run test:jest
 PASS  src/app/services/user.service.jest.spec.ts (12.524 s)
  UserService
    √ should be created (58 ms)
    √ should get user (20 ms)

Test Suites: 1 passed, 1 total
Tests:       2 passed, 2 total
Snapshots:   0 total
Time:        36.105 s

Looks like for Pact Tests promise is only the way forward, as done callback approach supported by Jest is not working.
For me as consumer, I am not sure if it's Jest issue or Pact! But that's fine. I can go ahead and use Promise in Pact tests.

I believe we can close this issue as original issue is resolved.
Thanks again for your help for spotting the root cause and providing solution.

@TimothyJones
Copy link
Contributor

I'm glad you've got it working.

However, I think you still haven't understood what I was saying above. Pact is compatible with done - you just have to make sure you only call it after the test has finished. To illustrate this, here's your test but rewritten with done callbacks:

    it('should get a User', (done) => {
      provider
        .executeTest(async (mockServer) => {
          console.log(
            `**** MockServer:: URL: ${mockServer.url}, ID: ${mockServer.id}`
          );
          service.setUrlPrefix(mockServer.url);

          return new Promise<void>((resolve, reject) => {
            service.get(userId).subscribe({
              next: (response) => {
                console.debug(response);
                expect(response).toEqual(expectedUser);
                resolve();
              },
              error: (error: HttpErrorResponse) => {
                reject(error);
              },
            });
          });
        })
        .then(
          () => {
            done();
          },
          (e) => done.fail(e)
        );
    });

As you can see, it's more wordy. If you prefer this way, then you could use this style instead.

Calling done isn't magic, you still need to make sure you understand the flow of the promises in your program, and only call it once the test is complete.

As an aside, this is a really good example of why done callbacks are discouraged by linters - it can be challenging to see the most appropriate place to put them. Personally, I agree with the linters - I would recommend instead returning promises; returning promises is a much more explicit way to write the test, and you don't need to remember to handle the failure case separately.

@TimothyJones
Copy link
Contributor

A second aside - I looked at your other, non-pact test, and you are not using done correctly there either. There could be a race condition that would result in the expect(httpClientSpy.get).toHaveBeenCalledTimes(1); test to either not be run (resulting in a pass that didn't check whether the spy was called), or to be run too early (resulting in a fail before the spy was called).

To fix this, I would recommend moving that expect call inside the callback.

This isn't jest support though, so I'll leave correcting the rest of the uses of done to you and your team.

@subodhgodbole
Copy link
Author

Thanks!! But IMO, main point of comparison was that -

  • In non-pact test - Jest waits till done() is called, which is done inside next, and Test succeeds.
  • But in previous implementation of Pact Test - Even though I had all expects calls inside next, test proceeded even before Pact server could process it.

Like I said this thread can be closed, as main problem with witting Pact Tests is resolved by using promise.

Thanks.

@pdeszynski
Copy link

I'm not sure whether this Issue should be closed. The same happens when jest-pact is being used. Pact fails when there are more than one tests using the same provider instance, for e.g. when running an example test - https://github.com/pact-foundation/pact-js/tree/master/examples/jest - from this repository it fails with

act between Jest-Consumer-Example and Jest-Provider-Example
    with 30000 ms timeout for Pact
      Dogs API
        ✓ returns a successful body (12 ms)
      Cats API
        ✕ returns a successful body (18 ms)

  ● Pact between Jest-Consumer-Example and Jest-Provider-Example › with 30000 ms timeout for Pact › Cats API › returns a successful body

    socket hang up

@mefellows
Copy link
Member

Hi @pdeszynski I think that is a separate issue from the one initially reported here.

Could you please open a new ticket with a reproducible example (the issue template should have this info) and we can look into it separately?

@TimothyJones
Copy link
Contributor

@pdeszynski are you using node 19? If yes, see #1066 for further information.

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

4 participants