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

Pending pacts and can-i-deploy #105

Open
holomekc opened this issue Aug 18, 2023 · 5 comments
Open

Pending pacts and can-i-deploy #105

holomekc opened this issue Aug 18, 2023 · 5 comments

Comments

@holomekc
Copy link

Hi,

I was wondering. We have the situation that a consumer created a new pact with a new state, which results in a pending pact. This works fine in our pipeline during the junit tests. We can see in the logs that the broker provided the pact, but the junit test ignores it because it is pending. So far so good.

The issue we face now is that the consumer deployed this changes already. This is breaking our pipeline because the can-i-deploy feature then tells us that we are breaking this pact.

I mean ok the consumer should not deploy. I get that part, but it also is a bit strange that our tests are fine with the pending pact and ignores it, but the can-i-deploy feature then complains that we are violating the pact.

Should not the can-i-deploy feature ignore pending pacts as well? We checked if we could ignore pending pacts in our pipeline manually. We are using https://docs.pact.io/pact_broker/client_cli/readme#can-i-deploy. But the issue is the json response does not include the pending pact information.

Do I have the wrong expectations here? What is your opinion?

Br,
Chris

@mefellows
Copy link
Member

It's a tricky situation. can-i-deploy is very much about telling you how reality works, whereas during development we have a bit more leeway. I suppose in this case, it is safe for the provider to deploy, because the provider already in the target environment (probably) doesn't work.

But, your consumer has broken the rules by ignoring this check. We could add more configuration/options, but it feels like it just makes the shooting of your feet more likely. This implies the tools have been circumvented, so you could collectively circumvent them again to resolve the situation.

This would be somewhat analogous to allowing you to regular git push after somebody else has git push --force a change to a repo. The --force was bad, and has now put the repo in a broken state, but allowing you to think things are normal by allowing regular options to keep working as is, is probably a bad idea.

@holomekc
Copy link
Author

Yeah I agree. We have some additional tests after the deployment we need and it is expected that every pacticipant can use one environment (test) and deploy and test ... let's say wildly ;). We include this environment in our can-i-deploy checks for faster feedback. I think this is our main issue at the moment. We do:
... deploy -> additional tests -> record-deployment -> can-i-deploy (everywhere)

We do it like that, because as said we need the deploy step on the test environment for the additional tests. This is why we did not put a can-i-deploy (test) infront of the deployment. Then it felt wrong not to include the record-deployment step afterwards. We were not sure where to include the can-i-deploy then. So we said ok at the end so that the teams still have the feedback from their tests etc. but also fast feedback from pact.

Maybe we can exclude test in can-i-deploy at the end, because as said we need to able to deploy there at any time and then maybe the can-i-deploy check makes no sense for this environment.

Or we can check if the usage of pending interactions can help us as well.

Nevertheless, I still want to ask regarding providing the information if a pact is pending or not also during the can-i-deploy step. Isn't that also a valid information, without changing anything regarding the can-i-deploy checks, but it provides us the info, ok somebody did something illegal or provides more flexibility for the different kinds of pipeline definitions?

Thx for the fast feedback btw.

@mefellows
Copy link
Member

I suppose a --allow-pending or something like that could be something for additional consideration. I'll move this to our feature request board, because it's separate to the specification itself.

@lophil
Copy link

lophil commented Aug 18, 2023

We actually ran into something similar where because we have pending pacts and WIP turned on, junit essentially ignores the failures. The problem is that unless the dev is intimately familiar with pact, they assume that everything is ok and could potentially merge a PR that is actually not going to let them deploy.

Since the failure is essentially “hidden” by junit, a PR pipeline could still pass and be merged. Especially true if something like auto merge is turned on

@holomekc
Copy link
Author

holomekc commented May 8, 2024

@mefellows
We have another similar situation. In an async pact we have System A, which is the provider of a message via SNS. And System B is the consumer of the message via an SQS subscribed to the SNS.
Our first thought was: "Ok due to pact we should be safe during deployment, because can-i-deploy will warn us and make sure, that System B needs to deploy first".

But due to the switched roles in async this is not the case. So can-i-deploy would force us to deploy System A first, which would result in message lost or if the SQS queue is already created in a queue, in which the message would pile up.

@mefellows are there any news regarding --allow-pending?

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

3 participants