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: support option to not fail if no pacts found #941

Closed
4 of 5 tasks
adamwitko opened this issue Aug 12, 2022 · 15 comments
Closed
4 of 5 tasks

Feature request: support option to not fail if no pacts found #941

adamwitko opened this issue Aug 12, 2022 · 15 comments

Comments

@adamwitko
Copy link

adamwitko commented Aug 12, 2022

Software versions

  • OS: Mac OSX 12.3
  • Consumer Pact library: @pact-foundation/pact@10.1.0
  • Provider Pact library: @pact-foundation/pact@10.1.0
  • Node Version: 18.7.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

When a provider does not find any pacts to verify the verification resolves with the relevant output describing the result. This means any test libraries do not fail as the promise is resolved.

Actual behaviour

When a provider does not find any pacts to verify for the consumer version selectors, an error inside the Rust pact_verifier is failing the verification run.

Steps to reproduce

  • Set your options to a standard looking configuration such as the following where there are no consumer pacts published that match the consumerVersionSelectors defined:
 const verifierOptions = {
      publishVerificationResult: false,
      provider: 'my-service',
      providerBaseUrl: 'http://localhost:8000',
      providerVersion: '09fe6815',
      providerBranch: 'my-branch',
      logLevel: 'debug',
      pactBrokerUrl: 'https://my-broker/',
      enablePending: true,
      consumerVersionSelectors: [
        { mainBranch: true },
        { deployedOrReleased: true },
        { matchingBranch: true }
      ],
    }
    
 await new Verifier(verifierOptions).verifyProvider()

Relevant log files

debug.log

@adamwitko adamwitko added the bug Indicates an unexpected problem or unintended behavior label Aug 12, 2022
@mefellows mefellows transferred this issue from pact-foundation/pact-js Aug 15, 2022
@mefellows
Copy link
Member

Moving this to the core.

This also came up in the slack forums recently.

@mefellows mefellows changed the title 10.X.X provider verification fails when there are no pacts to verify Feature request: support option to not fail if no pacts found Aug 15, 2022
@adamwitko
Copy link
Author

So sorry @mefellows I have no idea how I created it over here. My fault for having too many pact-foundation tabs open =(

@adamwitko adamwitko closed this as not planned Won't fix, can't repro, duplicate, stale Aug 15, 2022
@mefellows
Copy link
Member

Not at all - this was me (it needs to be addressed here first).

Screen Shot 2022-08-15 at 6 19 36 pm

Thanks for raising.

@adamwitko
Copy link
Author

Ok, apologise @mefellows, noob over here. Ah so is pact-reference the core of pact (rust, ruby versions) etc?

@mefellows
Copy link
Member

Exactly!

@TimothyJones
Copy link
Contributor

pact-reference is where the rust core is, but the ruby core I believe is elsewhere (pact-ruby-standalone and its dependencies).

@rholshausen rholshausen added enhancement and removed bug Indicates an unexpected problem or unintended behavior labels Aug 19, 2022
rholshausen referenced this issue in pact-foundation/pact-reference Aug 19, 2022
@rholshausen
Copy link

I've added a --ignore-no-pacts-error to the verifier CLI as well as a FFI function pactffi_verifier_set_no_pacts_is_error which takes a boolean value (pass zero in to disable the check).

I would like to point out that this option should be used carefully. If no pacts are found due to bad configuration, this would mean the verification build will pass, but no verification results will be published. Then the can-i-deploy check would fail, even though all the builds are green. This will be very confusing for people. Even worse, if can-i-deploy is not being used, then all the builds will be green but no verification has been done.

@mefellows
Copy link
Member

Awesome, thanks Ron. Noted on the risks, and I agree. It will be set to false by default. I'll move this to the Pact JS repo to implement from here.

@mefellows mefellows transferred this issue from pact-foundation/pact-reference Sep 2, 2022
@TimothyJones
Copy link
Contributor

What does can-i-deploy do if you ask about a provider with no consumers? I don’t think it should fail- it’s safe to deploy a provider with no consumers.

If it doesn’t fail, then I think pact verify with no consumers shouldn’t fail either (although I think it should warn). If pact verify fails when there are no consumers, then we can never end up deploying a provider with no consumers.

For these reasons, I prefer not failing when there are no pacts found by default. If it doesn’t fail, you don’t have to special case an initial deploy. If it does fail, you do.

@adamwitko
Copy link
Author

@TimothyJones this was my understanding of how the previous versions of @pact-foundation/pact-js behaved. I too had understood that it is absolutely fine for provider to exist with no consumer. That is an expected scenario, especially when teams are starting off with Pact or creating new provider services.

@mefellows
Copy link
Member

Yep, it's just that once you have consumers it's harder to detect this use case without additional checks. That could be worth exploring. e.g. If there are consumers of a provider, but no pacts are found, this is probably a configuration issue. If there are no consumers, then you'd always expect an empty list of pacts to verify.

cc: @bethesque

@adamwitko
Copy link
Author

@mefellows yeah it makes sense, it's a bit of a tricky one to get right with the same 'solution' for both issues.

@adamwitko
Copy link
Author

Hey is there any movement on this issue that anyone knows about?

This is stopping me rolling out v10 of this package as we cannot establish providers before there are pact for it to verify. It's a bit of a big blocker for our pipelines as we can't create consumer pacts before the provider exists and have them be deployable, and we can't create providers before the consumer pacts exists to verify with v10.

Can I help out at all?

@mefellows
Copy link
Member

Hi Adam. The Pact core now supports the feature via the set_no_pacts_is_error C interface (https://github.com/pact-foundation/pact-reference/blob/c128d22b838400d035ba900bedc4432bf98f1d5a/rust/pact_ffi/src/verifier/handle.rs#L220-L222).

We first need to expose this C interface to Node:

  1. A new method will need to be added to pact-js-core C inerface (https://github.com/pact-foundation/pact-js-core/blob/master/native/provider.cc and the header file)
  2. The new c interface will need to be registered and exposed to Node via https://github.com/pact-foundation/pact-js-core/blob/master/native/addon.cc

Then we can add the types to the core:

  1. A new optional parameter failIfNoPactsFound will need to be added to the verifier types
  2. A validation routine (copy any other boolean field validator) for the method: https://github.com/pact-foundation/pact-js-core/blob/master/src/verifier/validateOptions.ts#L189

Once released, this will make the type available to Pact JS :)

@mefellows
Copy link
Member

Closed, the PR above completes this feature.

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

4 participants