-
-
Notifications
You must be signed in to change notification settings - Fork 343
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
pass the pact dir to the server when writing pacts #79
Conversation
Thanks, I'll take a look over the weekend. are you able to elaborate a bit further on your use case? |
Hi, thanks for looking into my issue. My use case is that I'm integrating pact into my organisation's CI workflow. We use Jenkins to launch the tests, and the test environment is set up with As noted above, the pact server running in that docker image doesn't take the pacts dir as a constructor/setup option, it's passed as described in the gist you've found and commented on. If you don't pass it, then the described error occurs. I appreciate now that this may be a failure of the pact server, after your comment on that gist that the |
What I've seen is that on writing the pact, the server merges the post body with the default options to control how the pact is written. Therefore the I think that gives me enough information to work around the problem I had. I also think it means my fix in the PR would be sufficient, but that it is more likely the case that this is a bug in the server: letting the client control where files get written seems dangerous. I'll raise an issue over there. |
Thanks @iamleeg, this is a use case I had't considered. I'd like to think more on this one, however i'm leaning towards supporting this as an option. I can see the benefits. |
It makes sense to me to be able to pass in the option, as all the other similar options are configurable the same way. |
I can provide a default pact dir to the docker image if that would help. It will just be awkward getting the file back out again. But I guess it will stop the error happening. |
@mefellows notifications on gists don't work, so I didn't see your question :( The pact dir isn't part of the pact spec, it's an implementation detail of the ruby mock service. You can either provide it at startup via the command line, or when you tell the mock service to write the pact. |
I'm a bit confused because when I run |
I'll add the option in here too, marking as enhancement. Adding it to the docker container makes sense also. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once you fix the issues and have a green build, we can bring this in.
src/pact.js
Outdated
@@ -71,7 +71,7 @@ module.exports = (opts) => { | |||
|
|||
logger.info(`Setting up Pact with Consumer "${consumer}" and Provider "${provider}" using mock service on Port: "${port}"`) | |||
|
|||
const mockService = new MockService(consumer, provider, port, host, ssl, pactfileWriteMode) | |||
const mockService = new MockService(consumer, provider, dir, port, host, ssl, pactfileWriteMode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is probably the cause of the failing build. You're passing it as the third argument, but in the MockService constructor it's the 6th.
@mefellows I'm looking at removing the ability to set the location as per pact-foundation/pact-mock_service#74 |
I'd recommend NOT merging this PR - we will set the location in the docker image. However... I can't reproduce this problem with the docker image, so I'm very confused. |
@bethesque I also don't see that command failing. I don't know then what the difference between that request and the one I saw in the interaction with pact-js that I originally reported was, but will try to find out. |
I've reapplied my stash from the last time I was working on this, and now have a different failure earlier in the flow, so can't replicate this problem with the setup I was trying to use. I'm not sure what changed, I was still using pact-js 2.6.0 (and have also tried with 2.7.0 with no change). Unfortunately the new problem doesn't come with any logging beyond
so I have yet to dig into that. |
That problem was that I wasn't clearing out interactions between runs, so I can reproduce my original problem. I'm using
Looking at
} If I send that body to the docker image after running |
I've replicated the issue now - for some reason I needed to go through the full process of setting up an interaction to expose the issue. I've pushed a new docker image with the pact dir already set, so you should have no problems now. |
Thanks @bethesque! |
Hi @bethesque, sorry it took me a long time to clear the decks to test this again. I have updated my image with
and the behaviour has not changed. |
I can't reproduce the problem with the latest image. Can you clone
Can you see if you can modify |
The ruby
pact-mock-server
requires pact_dir to be supplied when writing pacts, otherwise this error is encountered:The JS client was not sending that option, expecting to start a
pact-node
mock itself and configure its pact dir on construction. This change adds that option, sopact-js
can be used with either mock service implementation.