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

Feature Request: Start provider on random open port #259

Closed
pk-nb opened this issue Jan 3, 2019 · 8 comments
Closed

Feature Request: Start provider on random open port #259

pk-nb opened this issue Jan 3, 2019 · 8 comments
Assignees

Comments

@pk-nb
Copy link

pk-nb commented Jan 3, 2019

Feature Request

Hi there! I'm opening up a feature request that I think will help make pact-js a lot easier to run with parallel test suites like in Jest. This is something we could really use to adopt pact across our Jest suite (already running in parallel). I'm happy to submit a PR with this but wanted to discuss just to make sure it is a welcome change.

I'd like to update the pact constructor with the ability to start the mock service on a random, unused port. pact-node already has support for this, but pact-js unfortunately does not forward support as it currently provides a default port. Even when explicitly set to port: undefined which does start pact-node on an unused port, it doesn't utilize it and thus fails to work when using other methods like addInteraction. We really would like to have this functionality to support an unknown number of mock servers across a parallelized suite. We tried to implement this in user space in our suite, but unfortunately it's really messy to implement a "port pool" on top of a server we only indirectly can access, especially when Jest runs all env files in their own process where we can't easily cleanup / release ports synchronously.

I think this can be a simple change, where we could let an undefined port be passed in and the pact-js layer can query for the port the mock service booted on. This would allow consumers to easily start several providers in a parallel suite. We would also want to resolve port as part of the setup promise to be used for routing client test requests to the right provider.

let provider;

beforeAll(() => {
  const provider = new Pact({
    port: undefined,
    pactfileWriteMode: "merge",
  });

  return provider.setup().then(({ port }) => {
    // use port to stub fetch code, route requests to that port, etc.
  });
});

I can implement as a non-breaking change, where we support a param such as port: undefined or port: 0 but still provide the default.

I know there are other proposed improvements to the API as mentioned in #215. I think this addition would make an API such as pactWith a lot easier to work with. Please let me know if this is a welcome change! Again, very happy to work on this to allow our team to use Pact. Thanks.

@pk-nb pk-nb changed the title Feature Request: Start on random open port and expose via method Feature Request: Start provider on random open port Jan 3, 2019
@mefellows
Copy link
Member

I'm all for this, particularly if you can do a non breaking change. #215 will probably be a much more wholesale upgrade.

@mboudreau
Copy link
Contributor

Yeah, I'm in the same boat, I definitely want this there since it would speed up testing drastically. It's one of the reason I made sure pact-node works well self contained :)

@TimothyJones
Copy link
Contributor

Yes please, I think this would be a great change, and an important first step for #215.

It would be nice if provider.setup() returned more options than just the port (eg, full base URL, or hostname and port separately).

@mefellows mefellows self-assigned this Feb 9, 2019
mefellows added a commit that referenced this issue Feb 9, 2019
- Pact.setup() now returns a PactOptionsComplete object,
  containing the full configuration of the Pact provider
- Minor refactor of the setup() function to dynamically
  load the port from the mock server

Fixes #259
mefellows added a commit that referenced this issue Feb 9, 2019
- Pact.setup() now returns a PactOptionsComplete object,
  containing the full configuration of the Pact provider
- Minor refactor of the setup() function to dynamically
  load the port from the mock server

Fixes #259
@mefellows
Copy link
Member

Ask and you shall receive v7.3.0 on the way out that both allows a dynamic port (see e2e and typescript examples) and will return the full complement of Pact options from setup().

Hope this is what you needed! :)

@pk-nb
Copy link
Author

pk-nb commented Feb 11, 2019

Hey thank you very much @mefellows! Appreciate you doing this ahead of me. Will give it a go with our team hopefully soon.

@mefellows
Copy link
Member

You're welcome. Let me know how it goes.

@Garcia-JuanLuis
Copy link

Hello, we have been using PactJs for almost two years now; and we are starting to run into port conflicts for branch builds, because we're specifying static ports in karma.conf.js. I came across this page and found it alleviating that there is a way to use random ports. I went ahead and removed the port property in the pact configuration section in Karma.conf.js; and I noticed that the mockedProviders are now starting with random ports. However, I'm having a heard time figuring out how to bind to the mockedProvider instances running on random ports from the test suites. The previous pactWeb object definition looked like this:
mockedCPA = new PactWeb({
consumer: '<consumer_name>',
provider: '<provider_name>',
port: 1234,
host: '127.0.0.1',
});

I want to be able to bind the above instance of PactWeb, to the random port associated with the mockedProvider.
I use Karma/Jasmine/Typescript.
Thanks in advance

@mefellows
Copy link
Member

Hi @Garcia-JuanLuis, could you please open this as a separate feature request rather than adding to the bottom of a closed issue? We can track it better that way - thanks!

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

5 participants