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

Multiple interactions per test case are no longer supported #1078

Open
5 tasks done
martinslota opened this issue Mar 28, 2023 · 14 comments
Open
5 tasks done

Multiple interactions per test case are no longer supported #1078

martinslota opened this issue Mar 28, 2023 · 14 comments
Labels
bug Indicates an unexpected problem or unintended behavior help wanted Indicates that a maintainer wants help on an issue or pull request

Comments

@martinslota
Copy link

martinslota commented Mar 28, 2023

Software versions

  • OS: Mac OS 13.2.1
  • Consumer Pact library: PactJS v 11.0.2
  • Provider Pact library: N/A
  • Node Version: v18.15.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

Multiple interactions can be added for a single test case.

Actual behaviour

The pact server crashes when adding the second interaction.

Steps to reproduce

Run npm ci && npm test in https://github.com/martinslota/pact-js-bug-multiple-interactions.

Relevant log files

failure.log

@martinslota martinslota added the bug Indicates an unexpected problem or unintended behavior label Mar 28, 2023
@TimothyJones
Copy link
Contributor

For others coming to this issue, The error message is:

[21:21:51.872] ERROR (13369): [email protected]: !!!!!!!!! PACT CRASHED !!!!!!!!!

The pact consumer core returned false at 'cleanupMockServer'. This
should only happen if the core methods were invoked out of order

This is almost certainly a bug in pact-js-core. It would be great if you could
open a bug report at: https://github.com/pact-foundation/pact-js-core/issues
so that we can fix it.

Can you try using the newer DSL instead of the 9.x one? Generally you shouldn't need more than one request per interaction, so there was probably an accidental assumption made that there would only be one when adding the compatibility layer for the older DSL.

The new DSL has less boilerplate - you don't have to call the lifecycle functions.

What are you doing that needs two requests in one interaction?

@mefellows
Copy link
Member

Thanks for digging Tim and the helpful repro @martinslota. I agree using the latter interface is recommended, however the idea was to support migrating from 9.x.x so it would be ideal to support it if we can.

@martinslota
Copy link
Author

Can you try using the newer DSL instead of the 9.x one? Generally you shouldn't need more than one request per interaction, so there was probably an accidental assumption made that there would only be one when adding the compatibility layer for the older DSL.

I switched to the newer DSL after I hit this problem, though that has then led to other difficulties, especially pact-foundation/pact_broker-client#131 and then some others which might be more on our side (I'm nowhere near done yet, so it's hard to tell).

The impression I got from the documentation was that the compatibility layer in version 10 (and 11, I suppose) would be almost fully backwards compatible with version 9. Not supporting multiple interactions is a deal breaker for the suite I'm migrating. I think that would be worth either fixing or clearly pointing out in the migration instructions.

@mefellows
Copy link
Member

Yes, I think it's a bug.

@effervescentia
Copy link

Hey @TimothyJones, I have a similar use-case

some of our code will retry the same request depending on the status that is returned (like a 503)

if we only return the failing response in a test then an exception is thrown after a number of attempts which doesn't really test the relationship between the two requests (which I would argue is part of the contract that is imposed on the provider in this case)

I could also see this being useful for testing a function that polls for a status. maybe it starts and then waits for an external job to complete and the job can go through multiple stages before completing

@mefellows
Copy link
Member

If this is in fact necessary to test, I would suggest these are tested as two separate scenarios differentiated by provider states (one for when the service is available and one for when it isn't).

The behaviour you want to test (retry mechanism) is actually the client's behaviour, and not the provider's, so I would further argue this isn't appropriate for a Pact test. The provider should not know how often a client attempts to retry a call.

@TimothyJones
Copy link
Contributor

I agree with Matt.

Making some assumptions about your implementation - the retry call wouldn't be visible to the business layer, so this test would take place in a lower level than the rest of the pact tests. If that's the case, I can see why it might feel appropriate to put it all in one interaction - so that the test boundary doesn't move.

However, there's some parts of the behaviour that would be more appropriate for individual tests. For example, the contract test won't include "does the client respect Retry-After", but if you treated them as separate interactions then this would just be part of the normal unit test.

It feels analogous to creating a resource that is immediately modified - where create and modify are separate interactions. Here, being told to retry later and actually retrying later are probably better off as separate interactions.

@kroupacz
Copy link

We also have a use-case where we need to add multiple interactions for one test. We are using "pact tests" (@pact-foundation/pact@^9.x.x) for our GraphQL API and it is not unusual for the GraphQL resolver to call multiple REST API endpoints in the background.

@mefellows
Copy link
Member

We also have a use-case where we need to add multiple interactions for one test. We are using "pact tests" (@pact-foundation/pact@^9.x.x) for our GraphQL API and it is not unusual for the GraphQL resolver to call multiple REST API endpoints in the background.

I think the general advice is the same here - i.e. stub the additional resolver calls.

mefellows added a commit to pactflow/example-consumer that referenced this issue Jun 26, 2023
@mefellows
Copy link
Member

mefellows commented Jun 26, 2023

OK so I thought this was suspicious - you can definitely test multiple interactions (I added it to the ID 10 exists scenario, apologies if that adds confusion).

Here is a change to an example project that shows it working: https://github.com/pactflow/example-consumer/compare/feat/repro-pact-js-1078?expand=1

➜  example-consumer git:(master) ✗ npm t

> [email protected] test
> cross-env CI=true react-scripts test

  console.log
    Product { id: '10', name: '28 Degrees', type: 'CREDIT_CARD' }

      at src/api.pact.spec.js:73:17

  console.log
    Product { id: 11, name: '28 Degrees', type: 'CREDIT_CARD' }

      at src/api.pact.spec.js:74:17

...

PASS src/api.pact.spec.js
  API Pact test
    retrieving a product
      ✓ ID 10 exists (186 ms)
      ✓ product does not exist (16 ms)
    retrieving products
      ✓ products exists (7 ms)

Test Suites: 1 passed, 1 total
Tests:       3 passed, 3 total
Snapshots:   0 total
Time:        5.125 s, estimated 6 s
Ran all test suites.

The test also fails if one of the calls wasn't made:

FAIL src/api.pact.spec.js (6.217 s)
  API Pact test
    retrieving a product
      ✕ ID 10 exists (200 ms)
      ✓ product does not exist (20 ms)
    retrieving products
      ✓ products exists (15 ms)

  ● API Pact test › retrieving a product › ID 10 exists

    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: /product/11
    	    Headers:
    	      Authorization: Bearer 2019-01-14T11:34:18.045Z

      at PactV3.<anonymous> (node_modules/@pact-foundation/src/v3/pact.ts:226:29)
      at step (node_modules/@pact-foundation/pact/src/v3/pact.js:33:23)
      at Object.next (node_modules/@pact-foundation/pact/src/v3/pact.js:14:53)
      at fulfilled (node_modules/@pact-foundation/pact/src/v3/pact.js:5:58)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 2 passed, 3 total
Snapshots:   0 total
Time:        7.736 s
Ran all test suites.

@martinslota
Copy link
Author

I believe that the original problem still stands with the original (not PactV3) provider.

@mefellows
Copy link
Member

Ah! You're right, my apologies. I'll re-open

@mefellows mefellows reopened this Jul 2, 2023
@mefellows mefellows added the help wanted Indicates that a maintainer wants help on an issue or pull request label Aug 18, 2023
@gorabin
Copy link

gorabin commented Feb 1, 2024

Any update on this ?

@mefellows
Copy link
Member

If there was an update, it would appear here ;)

This interface is quite old, so it's not a priority to go back and add this right now. If you are open to submitting a PR to address it, I could give you some pointers.

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 help wanted Indicates that a maintainer wants help on an issue or pull request
Projects
Status: New Issue
Development

No branches or pull requests

6 participants