search

ITV / scala-pact

A Scala implementation of CDC using the Pact standard
Other
108 stars 54 forks source link

Publish provider tests results to Pact broker #102

Closed garraspin closed 5 years ago

garraspin commented 6 years ago

Fixes #71

garraspin commented 6 years ago

I still need to add tests. But I wanted to initiate the conversation already. Manual tests look good

davesmith00000 commented 6 years ago

Hello! Really excited that you've had a go at this. I noticed you're still pushing to this PR - is it ready for review? Would you like to talk me through it a bit or give me a starting point? Otherwise I'll just try and wade through it. :-)

garraspin commented 6 years ago

Hi! Please proceed with the review. The functionality works and I would like to know your opinion. Basically I changed the provider tests so if the Pact contains _links attribute in the json it would keep those to retrieve the link where the results can be published to the pact broker. The pactBroker case class has a new Option[BrokerPublishData] parameter that if defined will be used to publish the results. Let me know how to contact you if you want more explanations.

garraspin commented 5 years ago

Hi @davesmith00000 , Were you able to have a look at the PR? Can you merge?

davesmith00000 commented 5 years ago

Hi, Sorry about the delay. Looks like it does the right thing. How are you testing it can I ask?

garraspin commented 5 years ago

I just added some unit tests for ResultPublisher. I am open to suggestions on how to tests the interaction with the broker on a integration test, if needed.

davesmith00000 commented 5 years ago

@garraspin You have not been forgotten, just hectic, I'm going to try and get this merged over the weekend.

davesmith00000 commented 5 years ago

Merged. I'm going to do some further verification before it goes into a release.

davesmith00000 commented 5 years ago

A colleague and I are in the process of testing this now. Nearly there, hopefully finished in the morning.

garraspin commented 5 years ago

Nice! Thanks for looking into it

davesmith00000 commented 5 years ago

Update: We've experienced a slight wrinkle in that one of the classes doesn't appear to be exposed to the *sbt files, but we're fixing it.

davesmith00000 commented 5 years ago

This is the specific error:

error: not found: value BrokerPublishData
      scalaPactEnv := ScalaPactEnv(Some("http"), Some("http://whatever/"), None, None, None, None, None, Option(BrokerPublishData("0.0.1", None)))
                                                                                                                                         ^
[error] sbt.compiler.EvalException: Type error in expression
[error] sbt.compiler.EvalException: Type error in expression

Looking at this now.

davesmith00000 commented 5 years ago

Right, fixed! You can now use BrokerPublishData in build.sbt. I realised too late that you'd added a nice helper method so that you didn't need that at all. Oh well. I've also verified that this works! I've published results from an external and internal verification. Need to update the docs now.

garraspin commented 5 years ago

Great that you fixed it! Thanks

davesmith00000 commented 5 years ago

It's not quite showing up on maven.org yet but you should be able to resolve version 2.3.3 now and try it. Thanks again for the work!