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

PACT's in the written JSON files are accumulated #59

Closed
Wisdomb33r opened this issue May 30, 2017 · 29 comments
Closed

PACT's in the written JSON files are accumulated #59

Wisdomb33r opened this issue May 30, 2017 · 29 comments
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@Wisdomb33r
Copy link

Hi,
I'm facing a very strange behavior. I may be missing something from the documentation but here is what happens.
I do have two backend services, I'll name them backend1 and backend2 as providers.
I do consequently have two test files for PACT's, which I'll name service1.pact.spec.ts and service2.pact.spec.ts.

Each of the file looks like :

declare function require(name: string);
const Pact = require('pact-web');

describe('Service1 PACT', () => {
  let provider;

  beforeAll(function (done) {
    provider = Pact({consumer: 'frontend', provider: 'backend1', web: true});
    provider.addInteraction({
      [...]
    });
  });

  afterAll(function (done) {
    provider.finalize();
  });

  beforeEach(() => {
    TestBed.configureTestingModule({
      providers: [FirstService],
      imports: [HttpModule]
    });
  });

  it('should return an a valid identifier when created', function (done) {
      [... inject and test]
  });
});

Versions used :

    "karma-pact": "0.0.7",
    "pact": "^2.5.0",
    "pact-web": "^2.5.0",

There is two backends with different provider names. Consequently, there is two files written, which is expected :

  1. frontend_backend1.json
  2. frontend_backend2.json

The fancy thing is the frontend_backend1.json is perfectly fine. However, the frontend_backend2.json do contains the interactions described in both pact.spec.ts files. I guess if I add PACT's for a third backend, I'll have a frontend_backend3.json file which contains the cumulative interactions described within the three files.

According to the readme file, the description of the finalize() function is :

Records the interactions registered to the Mock Server into the pact file and shuts it down.

I'm consequently expecting this call to reset the mocking context between the execution of different test cases because of the shut down.
Am I missing something ?
Thank you in advance for your help.

@mefellows
Copy link
Member

Hmm, that is strange. That being said, looking at that file you are using pact-web - is this a browser based test? (e.g. Karma/Jasmine)

If not, you just need to use the pact dependency, that should solve your problem.

pact-web is designed to automatically start a service before the browser tests start and stop it after the tests complete, it's this process that writes to the Pact file. pact doesn't have this restriction as it runs in a node-based environment.

@Wisdomb33r
Copy link
Author

This is indeed a Karma/Jasmine test in Angular (4.0.0) with TypeScript environment.
The configuration is directly taken from NomiJ example. The difference is I have two separate test files for different backends while NomiJ's tiny example only contains one.

@mefellows
Copy link
Member

OK, then I'm guessing (I'm not a Karma/Jasmine expert) that the pact plugin is only executed once before all tests are run, and the shutdown after they're all finished. This would explain the behaviour we're seeing.

Is there a way for the plugin to be executed once per test?

@bethesque - does this behaviour make sense to you from the perspective of the mock service?

@Wisdomb33r
Copy link
Author

If you are right, the mocking server is not shut down between tests files execution (contrary to what the documentation states). I've tried this code in my afterAll functions to "empty" the server instead of assuming it is shut down and restarted :

  afterAll(function (done) {
    provider.finalize()
      .then(function () {
        provider.removeInteractions();
        done();
      }, function (err) {
        done.fail(err);
      });
  });

I'm trying, after calling finalize and writing a JSON file, to remove all the interactions before going to the next PACT's test case. This code with the "done" callback ensures I'm going through the removeInteractions function. However, my problem is still present, the frontend_backend2.json file do contains the interactions of both backend 1 and 2 even if I called the mocking server to remove the interactions.

I placed logs in the beforeAll and afterAll functions, which are executed as expected in order (without any fancy order due to concurrency or anything) :

  1. LOG: 'beforeAll for backend1'
  2. LOG: 'afterAll for backend1'
  3. LOG: 'beforeAll for backend2'
  4. LOG: 'afterAll for backend2'

@Wisdomb33r
Copy link
Author

Also, even if the mocking server is not restarted and/or interactions removed, I do think the finalize() function should NOT write any PACT concerning backend1 in a file named frontend_backend2.json. Here is an extract of the final JSON PACT file for backend2 :

{
  "consumer": {
    "name": "frontend"
  },
  "provider": {
    "name": "backend2"
  },
  "interactions": [
    {
      "description": "a request for backend1",
      "request": {
        "method": "PUT",
        "path": "/.../........."
      },
      "response": {
        "status": 200,
        "headers": {
          "Content-Type": "application/json"
        },
        "body": {

        }
      }
    },
    [....] // other interactions concerning backend 2 which are relevant to this file
}

The PACT called "a request for backend1" is in the test file which starts with this pact initialization :

provider = Pact({consumer: 'frontend', provider: 'backend1', web: true});
provider.addInteraction({
      uponReceiving: 'a request for backend1',
      [...]

I would understand that calling finalize() in both test cases would make the files to be written twice. But mixing PACT's on different backends even if the provider initialization was performed with different providers parameters, this is indeed strange.

@mefellows
Copy link
Member

mefellows commented May 31, 2017 via email

@Wisdomb33r
Copy link
Author

Mocha is not a framework my company is using (I do not even know if they did evaluate it in the past). I have little knowledge on it.

There is a good point for browser-based solutions, the integration within our CI server may be easier than with a node-based environment. That's why the pact-web solution did seem an interesting one.

If I understood correctly how the pact technologies do work, there is three different components :

  1. pact version 2.5.0 which is the core library for PACT testing
  2. pact-web 2.5.0 which does contain an implementation of the browser-based mocking server
  3. karma-pact plugin 0.0.7 which is the way of making the two other interact within the context of karma/jasmine

Am I right ?
When you say the Ruby service should be enhanced, does that mean the problem lies within the implementation of the pact-web library ? Or a transitive dependency of this library ?

@mefellows
Copy link
Member

I would have though the opposite - browser based tests are notoriously more difficult to manage than simple Node based ones (you need a browser, for one).

In any case, that's not the point.

Yes, it's a transitive dependency and you are right with those components. Only in browser-based environments do you need 2 and 3 (hence my suggestion). Under the hood, we use a few Ruby gems that have all of the Pact matching and verification logic, so we need to fix it upstream.

Long term, this will be replaced by a native module but that is a lot of work and will take some time to implement.

In terms of tactical solutions for you, is there are way you can execute your Karma tests in separate sessions for each Consumer<->Provider pair?

I had a brief look at the Karma docs, and couldn't see any obvious API/Dependency that I could hook into in karma-pact to properly shut down servers in between test suite runs.

@Wisdomb33r
Copy link
Author

I discussed this point internally and we chose to use the karma/jasmine solution as-is. The consumer PACT tests seems ok, just the generated files are broken when there is multiple backends within the same mocking server run.

We were NOT planning to publish automatically our JSON PACT's to a broker. Instead, we do copy our generated JSON manually to commit it to our backend project (and in fact, we have to translate it to Spring CDC DSL which is not fully compatible with the pact foundation DSL).

With this in mind, it is not a big problem to force the execution of the Karma PACT tests one-by-one, which solves the problem of having multiple backends PACT's written to the same files.

I'll keep an eye on the pact-web project to see if newer versions are coming (and hopefully one that solves this bug).

@mefellows
Copy link
Member

Thanks @Wisdomb33r. We'll see what we can do.

@bethesque does this make sense to you? The way Karma works is that we can only have one hook before/after all tests are run to start and stop a mock server.

This means requests to all providers go to the same instance. Even if they have different providers in the requests, they all get tacked into the same pact file (see trail above). Anything we can do, you think?

@bethesque
Copy link
Member

There's another way to run the mock servers, where you start one control server, and it takes care of starting and stopping various mock services for you, and shutting them down again. It uses the name of the provider in the incoming interaction to work out whether it has a mock started already, and then starts one if it hasn't, and sends back the Location of the new server in the response.

https://github.com/pact-foundation/pact-mock_service/blob/master/lib/pact/mock_service/cli.rb#L31

It's been a while since I've used it, but I think we could make it work.

@mefellows
Copy link
Member

mefellows commented Jun 2, 2017 via email

@bethesque
Copy link
Member

Or rather, I know it did use to work, because I've used it before with JS, but that was 2 years ago when I still had an idea what was going on in the JS implementation. It's all greek to me now.

@mefellows mefellows added the bug Indicates an unexpected problem or unintended behavior label Jun 6, 2017
@thombergs
Copy link
Contributor

We're running into the same issue. Since we're depending a lot on automation in our microservice enviroment we must publish our pacts to a pact broker from our CI build automatically, which is not possible currently.

Also, when a test that creates a pact is run twice in the same Karma session, we get the following, which feels like a different symptom of the same issue:

Error: Failed: An interaction with same description ("a request for creating a Branch") and provider state ("allows creating of a Branch") but a different request body has already been used. Please use a different description or provider state.

...so we have to kill karma and run the tests again as a workaround.

We're happy to serve as friendly testers as soon as you come up with a possible solution :).

@bethesque
Copy link
Member

@thombergs strange that the request bodies are different. Is there something dynamically generated in there? You can do identical interactions over and over, and the duplicates are discarded, but when you try and add an interaction with the same description and state that isn't identical, the mock service won't let you, as the description/state are a unique key.

@thombergs
Copy link
Contributor

@bethesque OK, I should have read the error message more carefully. Yes, we use randomly generated test data for each request. When using static test data it works as expected. Thank you very much for the quick response!

The issue that the interactions add up in the resulting pact files still exists, however. Each created pact file contains all interactions from all pact tests that were run in the same karma session, only in a different order.

@bethesque
Copy link
Member

What would you expect it to do?

@mefellows
Copy link
Member

@thombergs thanks for offering to test any changes!

If all of the interactions are being added to the same pact file for a given consumer-provider pair then that is the expected behaviour. Unless they are accumulating between runs of the same tests (see pactfileWriteMode flag to configure this behaviour), or you have multiple providers.

If the later, there is a discussion over in karma-pact where we will be introducing the ability to have multiple providers setup in your karma tests which should resolve this issue. Please watch this space.

cc: @Wisdomb33r

mefellows added a commit that referenced this issue Sep 18, 2017
- For pact-web use cases, where mock service is started
  independently of the test cases themselves, we allow
  consumer and provider to be specified in underlying
  mock service directly.
- Logs warning to console if details aren't provided to
  assist in debugging issues
- Addresses pact-foundation/karma-pact/pull/4
- Addresses #59
@thombergs
Copy link
Contributor

Yep, we have mutliple providers. I'll check out your change in 3.0.0.

@Wisdomb33r
Copy link
Author

Same as @thombergs , we have multiple providers (micro services in our case). And the resulting JSON files (one per provider) contains interactions of all the providers (at least the latest JSON written).
I'll also stay tuned for the introduction of multiple providers setup to test it.

@mefellows
Copy link
Member

3.0.1 was just released for pact and pact-web, and also 1.0.0 of @pact-foundation/karma-pact was released earlier today.

You feedback would be much appreciated.

@thombergs
Copy link
Contributor

Thanks for the update. I got it to work in our Angular application after I finally understood the configuration correctly. Here's what I did in case anyone else has the same problems:

  • include "@pact-foundation/karma-pact": "~1.0" and "pact-web": "~3.0" as devDependencies
  • in karma.conf.js add a mock server configuration for each consumer / provider pair like this
    pact: [{
        cors: true,
        port: 1234,
        consumer: "ui",
        provider: "provider1"
      }, {
        cors: true,
        port: 1235,
        consumer: "ui",
        provider: "provider2"
      }]
  • in karma.conf.js add a proxy for each provider
    proxies: {
       '/provider1/': 'http://127.0.0.1:1234/provider1/',
       '/provider2/': 'http://127.0.0.1:1235/provider2/',
     }
  • in a pact consumer test add the port to the correct mock server
    beforeAll(function (done) {
     provider = Pact({
       consumer: 'ui',
       provider: 'provider1',
       web: true,
       port: 1234
     });
    });

The pact files are generated into the main folder now. Before the update they were generated into the "pacts" folder. Is there a way to configure the target folder.

@mefellows
Copy link
Member

Thanks for sharing this @thombergs!

I'm going to close this off for now.

@Wisdomb33r - please re-open if you have issues.

@gabrielalan
Copy link

gabrielalan commented Sep 6, 2018

Ok, I got the solution, but this is very odd. In my case I have 30 and something microservices running in the BE, and I would need to specify 30 servers configuration to run in my unit tests, which would probably make it slow AF, let alone the fact that I need to come up with unused ports to use in the configuration. No other solution for that? I mean, that doesn't make any sense for me actually, to accumulate everything in the contract files...

Is there a way to configure pact-web like @bethesque said, with a "Control Server"?

@mefellows
Copy link
Member

@gabrielalan OK first, cool your jets mate - we hear you :)

Configuration

In my case I have 30 and something microservices running in the BE, and I would need to specify 30 servers configuration to run in my unit tests

So if you weren't using Pact you probably would have had at least 30 unit tests and have made 30 mocks/stubs for each of those services. I'm not saying we like configuration, but the configuration required for this is hardly onerous in comparison to the alternative. It's also very explicit, which in unit test frameworks can be a very good thing.

Speed

which would probably make it slow AF

Does it, or are you just assuming? What makes it slow specifically (so we know how to fix it with alternative solutions)?

I could see how this might be slow when starting a test suite waiting for all of the servers to come up, particular in frameworks like Karma.

Ports

let alone the fact that I need to come up with unused ports

There are literally tens of thousands of ports you can run this on, what's the problem specifically? In CI environments you should mostly be able to control which ports are and aren't available - this is especially true with modern tools running in cloud or dockerized environments.

Summary

So i'm not necessarily convinced of the problem (there is likely multiple, with multiple solutions), or that we're going to be able to solve it simply.

The control server option sounds like an approach that could work here, however it will require significant refactors across pact-js and pact-node libraries. If I understand correctly, we still need a feedback mechanism from the control server to tell each test suite which port maps to each service (as it still creates a new mock process in the background) - this could be a sticking point for Karma, as the configuration needs to be known in advance of starting the suite

I'm not ruling it out, but the effort would need to justify the reward. If you're open to spiking some approaches I'd be happy to point you in the right direction?

@gabrielalan
Copy link

I see... I'm just duplicating each server configuration for each provider, so the start time is something that adds up 1,5 minutes to our CI jobs. I'll take a look to see the reason behind it.

Besides this, for me the problem is: we could have only 1 server, that actually saves the contract for each provider separated, not mixing up all the providers pacts inside all the contracts. We can workaround this creating multiple mock servers, but this sounds dirty to me. That's just my point of view. I probably am not aware of the implications of having this solution to have only 1 mock server...

@mefellows
Copy link
Member

If you have a single mock server, how do you determine which "provider" each request is supposed to be sent to?

we could have only 1 server, that actually saves the contract for each provider separated, not mixing up all the providers pacts inside all the contracts

Can you please elaborate? Do you want it to spit out a single JSON file for all of the interactions for a consumer or 1 JSON file per consumer-provider pair (current behaviour)?

@cjrogala
Copy link

cjrogala commented Sep 28, 2018

UPDATE: If you run into an issue while debugging a test and then stop the test, the service will continue to run. This caused the below issue that was resolved by end the ruby...exe task. Is it possible to configure the test to dispose of a mock service if it is found in the creation of the service? Any direction would be helpful

Original:
It looks like the mock server is not shutting down the port once all the test have completed. In my project, I had the service consumer client returning one type of model, but then changed it to another one. This caused the body to change. This resulted in the error:

PactNet.PactFailureException : An interaction with same description ("A GET request to retrieve the [name of the resource]") and provider state ("There is an [name of the resource] with id 'tester'") but a different response body has already been used. Please use a different description or provider state, or remove any random data in the interaction.

Additionally, after all the tests executes, I am still able to interact with the server within PostMan. I would think this is not ideal on a build server since various builds from difference efforts could impact one another. I might be able to get around this with configurations, but it's going to be a bit of work.

I'm just prototyping right now, so I might be missing something.

I've tried the following:

  1. I tried calling _mockProviderService.Stop(); in the constructor of the test to see if it stopped the server, but that did seem to do it.
  2. In the constructor, I call _mockProviderService.ClearInteractions(); and when the line that registers an interaction gets called, the error appears.
  3. I tried deleting the pact json file that is generated, and that didn't do anything.

My expectation is that after all the tests run, the mock server port is disposed of. To verify this, I should be able to call the mock server's base URI in PostMan and the response should indicate the service was not found.

Right now it is returning:

Status Code: 500
Body:
{
"message": "No interaction found for GET /",
"interaction_diffs": []
}

@mefellows
Copy link
Member

@cjrogala your issue description seems to be unrelated to the JS implementation (assuming Pact NET?).

In any case, generally speaking the mock server should be shut down between test suites, but not between test cases. Each Mock Server that is started is specific to a consumer and provider, so there is nothing wrong with leaving it around for the lifecycle of those particular tests. At the end of your test execution process, you definitely will want the server to shutdown to avoid dangling processes, port conflicts etc.

You shouldn't have to shut down the process yourself, that sounds like either a) a bug or b) an enhancement that could be added to an interface if not by design.

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

6 participants