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

feat(generators): Add ProviderState generator & others #273

Closed
wants to merge 2 commits into from

Conversation

slt
Copy link
Contributor

@slt slt commented Sep 15, 2022

This is required to support using provider state generators in pact-provider-verifier
pact-foundation/pact-provider-verifier#85

Requires pact-foundation/pact-support#97

My work is based on @hhhonzik's earlier pull requests

pact-foundation/pact-provider-verifier#53
#209

I have never written Ruby before so please forgive if it's not very idiomatic - happy to action any feedback given.

@slt slt changed the title feat(generators): Pass back result from provider state setup URL feat(generators): Pass context and add ProviderState generator Sep 15, 2022
@slt slt marked this pull request as ready for review September 16, 2022 02:01
@slt slt changed the title feat(generators): Pass context and add ProviderState generator feat(generators): Add ProviderState generator & others Sep 21, 2022
@bethesque
Copy link
Member

Sorry for the slow reply. Thank you so much for this! @uglyog can you have a look at this and see if it's inline with how the other implementations work please?

@bethesque bethesque requested a review from uglyog September 30, 2022 01:28
@slt
Copy link
Contributor Author

slt commented Oct 19, 2022

It requires pact-foundation/pact-support#97 in order to pass the tests

I have been using this as part of my own build of pact-provider-verifier successfully for the last month

I would welcome any feedback and am happy to action any changes

@rholshausen
Copy link

Looks good to me

@slt
Copy link
Contributor Author

slt commented Nov 10, 2022

If we could please get pact-foundation/pact-support#97 (which is only minor) merged over in that project, then this pull request should start passing tests here🤞

@bethesque
Copy link
Member

Have released pact-support.

@slt
Copy link
Contributor Author

slt commented Nov 15, 2022

Thank you! I've updated the pact.gemspec to use the new pact-support version

@slt
Copy link
Contributor Author

slt commented Nov 15, 2022

Looks like it has passed 🎆 🥳

@bethesque
Copy link
Member

@slt I'm sorry it's taken so long to get this in. I don't have a lot of extra bandwidth at the moment. I really appreciate your contribution!

Before we merge this in, we need to work out how this is going to be supported. Are you willing to be available to answer community questions about this and fix any bugs, because I won't be able to help with this. If so, it would be great if you could be in the #pact-ruby channel in our slack workspace if you aren't already. It also needs to be documented. Would you be happy to pick that up?

Again, really really appreciate your work on this!

variable = buffer[position + 2...end_position]

if !params[variable]
logger.info "Could not subsitute provider state key #{variable}, have #{params}"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this should be an error/exception

it is very frustrating when this happens

@slt
Copy link
Contributor Author

slt commented Nov 16, 2022

Hi @bethesque

I am more than happy to do the documentation and answer any questions or do bug fixes in the short term, but this is the first time I've written anything in ruby so I'm not really too knowledgeable or invested in ruby long term, unfortunately.

I have mostly written this pull request (selfishly) to unblock myself and to get the feature into the standalone pact-provider-verifier via pact-foundation/pact-provider-verifier#85 for use in my organisation's Go and PHP projects, where I am pushing for us to adopt Pact.

Also, I have not really been focused on the use case of those using this library directly in their ruby projects, I think there is additional work to be done for it to make total sense there.

From the matrix on https://docs.pact.io/roadmap/feature_support and from my understanding

This pull request should tick the box for:

  • Injecting values from provider state callbacks
  • Pact specification v3 generators

But to be able to generate contracts that use any of these features in pact-ruby you also need:

  • Pact specification v3 matchers

I'm going for a long break over December, so happy to hold off and revisit in the new year if that works better. I've been idling in #pact-ruby but have said hello today :) Thank you for taking a look

@slt
Copy link
Contributor Author

slt commented Nov 16, 2022

On a side note, I just found pact-rust's pact_verifier_cli. Am I better off using that instead of trying to improve pact-provider-verifier?

I am not really tied to a language, I am just looking for something I can run independently from a docker container

Does that already do what I'm trying to achieve here?

@bethesque
Copy link
Member

Am I better off using that instead of trying to improve pact-provider-verifier?

Yes!

Sorry, I thought you were doing this for the Ruby native implementation. I would have let you know about the rust verifier if I'd realised!

@YOU54F
Copy link
Member

YOU54F commented Mar 8, 2023

Thanks @slt for your work on this, appreciate that you mentioned you are not a Ruby developer and so providing support for it would be tricky.

I will in the meantime invite anyone who wants to review to add comments or support for this it would be much appreciated, and if not, we will endeavour to get this out and documented when we can :)

@YOU54F
Copy link
Member

YOU54F commented Aug 14, 2024

closing in favour of #320, thanks Steve

@YOU54F YOU54F closed this Aug 14, 2024
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

Successfully merging this pull request may close these issues.

5 participants