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

Introduce Pact contract tests #164

Closed
edeandrea opened this issue Oct 18, 2022 · 13 comments · Fixed by #165
Closed

Introduce Pact contract tests #164

edeandrea opened this issue Oct 18, 2022 · 13 comments · Fixed by #165
Assignees
Labels
Automation Automation enhancement New feature or request fights-service Fights service heroes-service Heroes service villains-service Villains service

Comments

@edeandrea
Copy link
Collaborator

edeandrea commented Oct 18, 2022

@holly-cummins & I had a talk at Devoxx Belgium about Pact contract testing:

We used the Quarkus superheroes app as the foundation for our demos. Currently all the code resides in my own personal fork. We need to get this into the main repo.

The code, as-is, will not break anything, but it also will not be executed during any CI/CD process, or dev mode/continuous testing.

There are a few caveats that need to be sorted out.

  1. The Pact tests use the free tier within Pactflow broker - https://quarkus-super-heroes.pactflow.io.
    • There is a CI/CD robot user that has credentials we can store as GitHub secrets, but the larger problem is that the account we currently use is tied to my (@edeandrea's) redhat.com email address. Ideally we should have a more "generic" user to own the account.
    • Under an account you can assign individual users with permissions. @holly-cummins and I are currently in there as admin users.
    • @cescoffier / @maxandersen how do you want to handle this? I can add you both as users in the current Pactflow and you can explore, or I can demo it for you so you get an understanding of it. Let me know your thoughts.
      • You need to authenticate with the broker even to perform read actions
  2. The Pact tests as they currently are written will not run by default - they have to have flags to enable them
    • This is because, in its current state, Pact does not work in dev mode/continuous testing. There are some issues on the Pact side and some work on the Quarkus side
    • This way we can safely merge in the code without breaking any of the existing CI/CD workflows and/or dev mode/continuous testing within any of the apps
    • @holly-cummins has already started work on that. In our Devoxx demo we showed it working in continuous testing on the provider side. That was done with a local patch to Pact and the beginnings of a Quarkus extension which @holly-cummins is building
  3. From a CI perspective we should hook in, at a minimum, the provider verification tests into the simple build/test workflow AND the nightly CI process that tests against Quarkus main
    • The simple build/test workflow runs against all PRs as well as the first workflow against any pushes to main.

@holly-cummins did I miss anything?

@edeandrea edeandrea added enhancement New feature or request fights-service Fights service villains-service Villains service heroes-service Heroes service Automation Automation labels Oct 18, 2022
@edeandrea edeandrea self-assigned this Oct 18, 2022
@maxandersen
Copy link
Member

What's the issues that needs fixing on either side? Got links?

@edeandrea
Copy link
Collaborator Author

edeandrea commented Oct 18, 2022

One issue is not a technical one - its about sorting out credentials in the Pactflow broker. Not sure having an account owned by my @redhat.com email address is a good idea. Would be good to have some kind of "organization" account or something that owns the parent account, then can add delegated users to those who need access, as well as a system CI account.

@edeandrea
Copy link
Collaborator Author

edeandrea commented Oct 18, 2022

Oh wait you mean the Quarkus & Pact issues :) Its all about classloading & classloader issues. @holly-cummins would be able to speak better to that since she's been the one doing the digging on both sides.

@holly-cummins
Copy link
Collaborator

I think you got them all, @edeandrea , apart from the issues on https://github.com/holly-cummins/scratch-pact-extension which are mostly future enhancements, rather than defects.

I need to do the paperwork to get the extension repo(s) set up so we have somewhere proper to hang work items. For example, we realised that on the consumer side, using a dev service for the Pact broker is a great use case for dev services and fixes a pain point in how Pact handle flipping between local and remote contracts... but I haven't raised that yet (doh).

@holly-cummins
Copy link
Collaborator

holly-cummins commented Oct 18, 2022

The dev service for the pact broker would fix the credentials issue @edeandrea identifies above. However, getting the right behaviour for a provider-side broker dev service would need a bit of thought.

(As a temporary measure while we wait for someone (cough) to write the dev service, we could switch to local pacts. It's less visual for a demo, but works out of the box. It's then a one-liner for an experienced user to switch to the broker.)

@edeandrea
Copy link
Collaborator Author

The dev service for the broker for local dev would be nice, but I'm not sure it solves the whole problem. I still think we need the "real" broker to store contracts in for when the CI workflows run.

@edeandrea
Copy link
Collaborator Author

(As a temporary measure while we wait for someone (cough) to write the dev service, we could switch to local pacts. It's less visual for a demo, but works out of the box. It's then a one-liner for an experienced user to switch to the broker.)

That would mean then that we'd have to order the CI workflows so the consumer built before the provider.

I envision some kind of manual push (for now) to the broker, that way the consumer can run it's tests during CI and the providers can do their verification pulling from the broker.

The CI is a GitHub action matrix job, so everything builds in parallel against multiple JVM versions, so trying to share output from one parallel build to another would be tricky.

@holly-cummins
Copy link
Collaborator

I think ideally CI shouldn't depend on the pact broker, even if we use it in demos. It feels like an extra dependency which isn't strictly necessary.

The parallel-builds situation could be half-solved by having the consumer check the contract into the provider's CI. That's a standard pattern, and it would work mostly-fine. The case it breaks is when one side made a breaking change and it took two CI cycles for the new contract to get through the wash to the other side.

Alternatively, maybe there's a github actions mechanism to feed the output of one job into the input of another that would force the correct ordering without too much work on our side.

@edeandrea
Copy link
Collaborator Author

I think we should talk about how we want CI to work. (Yes consumer and provider teams actually need to talk to one another :) )

For now, I'll start a PR and at least get the code in so people have it to look at.

I'd still like to have the broker too, but we can figure out how to do that.

@edeandrea
Copy link
Collaborator Author

This is the CI - https://github.com/quarkusio/quarkus-super-heroes/blob/main/.github/workflows/simple-build-test.yml

Since the jobs are matrix jobs, wouldn't it be difficult to order the jobs properly since the jobs themselves are generated by the matrix? Unless there is something built into GitHub actions that can do that?

edeandrea referenced this issue in edeandrea/quarkus-super-heroes Oct 18, 2022
@edeandrea
Copy link
Collaborator Author

@holly-cummins I've opened #165 with the initial code for both the consumer & provider. For now it is exactly what I had on my local fork using the broker. We can iterate on it as we shake some things out.

@edeandrea
Copy link
Collaborator Author

edeandrea commented Oct 19, 2022

So @holly-cummins and I had a chat offline about what the right thing to do here is and we decided that this should be broken into 2 pieces:

  1. Get something into the superheroes that is easy to show and easy to reproduce for anyone who is just cloning/forking the repo and running the tests. Anyone cloning the repo and running the tests shouldn't have to make any changes in order for things to work.
    • This means that we shouldn't use the Pact broker - instead just check the contract into the provider's source tree.
    • Fully document in the README's whats being done and how someone might switch it to use their own broker if they wanted to.
    • This way the Superheroes CI can also run both the consumer & provider contract tests as part of both PR verification builds, commits to main, as well as the nightly build against Quarkus main.
    • The Pact tests, though, would still need to be "opted into" because they would break dev mode/continuous testing. This is something the Quarkus extension would need to fix.
  2. In the future, once an "official" Quarkus extension evolves, revisit this setup and see if anything can be done differently.

Did I miss anything @holly-cummins ?

I'll push a few more commits to #165 with some changes to move away from the broker by the end of the week.

edeandrea referenced this issue in edeandrea/quarkus-super-heroes Oct 19, 2022
edeandrea referenced this issue in edeandrea/quarkus-super-heroes Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Automation Automation enhancement New feature or request fights-service Fights service heroes-service Heroes service villains-service Villains service
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants