Use quarkiverse pact extension

[holly-cummins]

Now that there are releases of both the consumer and provider pact extensions, it makes sense to switch the superheroes sample to use them.

For now, the extension just gives feature parity with 'normal' pact, but it will continue to add quarkus joy in subsequent versions. I think the extension will also be needed for Pact to work in Quarkus 3.

Although dev mode doesn't work with the extension (see quarkiverse/quarkus-pact#28), there is a workaround. In the pom.xml, changing the scope of the dependency to default (or provided) allows continuous testing to work.

    <dependency>
      <groupId>io.quarkiverse.pact</groupId>
      <artifactId>quarkus-pact-provider</artifactId>
      <version>${pact.version}</version>
     <!--  <scope>test</scope> -->
    </dependency>

This workaround might no longer be needed in Quarkus 2.16, although in my local testing it still is. Hopefully it definitely not be needed in Quarkus 3.

[edeandrea]

Actually my mistake. Looks like rest-villains was missed.

We should also update the documentation for all 3 as well in the contract testing section for each.

[edeandrea]

@holly-cummins just to summarize our offline conversation here:

1. rest-heroes also needs to be changed to use the new quarkus-pact-provider extension
2. Change the <scope> for all quarkus-pact-provider & quarkus-pact-consumer dependencies to be provided
   • Semantically, this isn't the right thing to do, but technically the benefits outweigh the semantics.
     • Doing so allows dev mode/continuous testing to work as well as removes the need for the guards (-DrunContractVerificationTests=true and -DrunConsumerContractTests=true flags)
3. Remove the @EnabledIfSystemProperty guards in any tests that read the runContractVerificationTests (3 instances, 1 each in rest-fights, rest-heroes, & rest-villains) or runConsumerContractTests (2 instances in rest-fights) flags
4. Remove any/all references to the runContractVerificationTests and runConsumerContractTests flags in the documentation, code, as well as the GitHub action automation.
5. Update the documentation/READMEs in all 3 projects.
   • Currently there is a mention/link to the extension. Let's make any necessary updates to the docs based on the extension
   • Maybe even explain the use of the provided scope & the reasoning behind it?

Did I miss anything?

[holly-cummins]

Thanks for the detailed notes, @edeandrea .

All that is now done, and pushed. I didn't squash, to make re-review easier (and because there were relatively few overlapping changes).

I’ve updated to switch over the missed module, and also set the dependency scope to provided so that dev mode works.

However, getting dev mode to work for the consumer tests turned out to be non-trivial.

The presence of @testprofile on the consumer tests prevents Pact from working properly. I’ve raised quarkiverse/quarkus-pact#58. Instead of using test profiles, we can just set the pact port to be the same as what’s configured in application.properties … but only if HeroesVillainsWiremockServerResource doesn’t update the application config.

I tried configuring wire mock to use a static port, and hardcode that same port in the consumer tests, but it didn’t work. The reasons weren’t clear (quarkiverse/quarkus-pact#60). It seems like perhaps the issue isn’t port confusion, and it’s a more fundamental one where any lifecycle or profile interception interfering with pact.

The profile issue looks like it will need Pact or Quarkus code changes to resolve, so for the moment I’ve thrown in the towel and re-disabled the consumer tests. This is annoying, but we’ll get there in the end. I implemented a more granular disabling logic, so that the consumer tests won’t run in dev and test modes, but will run in normal mode, without needing to set system properties. I’ve confirmed the tests do run (and are capable of failing) in normal mode.

Instead of the annotation, I could have just used the quarkus.test.exclude-pattern configuration property to run the tests in normal mode and skip them in continuous testing.

I’ve also removed the junit5-mockito dependency, since nothing seemed to be using it.

[holly-cummins]

Actually, quarkus.test.exclude-tags would be even cleaner than the name, as a way to exclude tests in continuous testing. If I didn't hope the exclusion was very temporary, I'd go back and do it that way.

[edeandrea]

I'll take a peek at the changes in a bit. No worries about squashing - I always squash when I merge and actually prefer multiple commits on a pr. Easier as a dev IMO and easier to review what's changed. Those who say all PRs should only have a single commit - not sure what land they're living in. That's what squashing when merging does :) but alas I digress :)

The only reason for the test profile was to re-configure stork to point to the pact mock server rather than wiremock (or the real app port). The pact consumer tests need to run against the pact mock server.

I tried at one point to use use a QuarkusTestResourceLifecycleManager to alter the config for those tests but I remember running into issues with that, but don't quite remember what specific issues I had.

[edeandrea]

Looks good @holly-cummins ! Once the current checks complete I'm going to update with the base branch & let them re-run. If all goes well I'll merge then.

Thank you very much for this!