Skip to content

Conversation

@loicmathieu
Copy link
Contributor

Fixes #10364

I'll open a PR on the quickstart to add this later.


[source,java]
----
private static WireMockServer wireMockServer = new WireMockServer();
Copy link
Contributor

Choose a reason for hiding this comment

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

I am pretty sure that we want to do something like

public class WiremockCountries implements QuarkusTestResourceLifecycleManager { // <2>
instead of this per pest configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bot approache are good for different reason.
If we prefere TestResource I'm OK to switch to it inside our integration test and update the guide.

Copy link
Contributor

@geoand geoand Jul 1, 2020

Choose a reason for hiding this comment

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

Not really, when you use %test.quarkus.oauth2.introspection-url=http://localhost:8080/introspect it applies to all tests, so best make sure that you have a Wiremock server up for all tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a good point indead ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

:)

@loicmathieu loicmathieu force-pushed the doc/oauth2-mock-authorization-server branch from a3cca08 to 6956c3c Compare July 1, 2020 12:23
@loicmathieu
Copy link
Contributor Author

@geoand I switch to using a test resource instead of starting Wiremock directly inside the test. I must admit it is way elegant (I love the fact you can override runtime properties, learn a new trick ;) ).

I'll open a quickstart PR to include this inside the quickstart with a test.

@geoand
Copy link
Contributor

geoand commented Jul 1, 2020

Excellent, thanks a lot @loicmathieu!

I'll take a quick look later on, but I'll let @sberyozkin have the final say.

@loicmathieu
Copy link
Contributor Author

And the PR that update the quickstart is here: quarkusio/quarkus-quickstarts#609

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

LGTM

@loicmathieu loicmathieu force-pushed the doc/oauth2-mock-authorization-server branch from 6956c3c to 3765f3b Compare July 1, 2020 15:55
Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

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

This is just super cool, thanks @geoand for doing the real review :-)

@loicmathieu - thanks

@sberyozkin
Copy link
Member

I'll follow Loic's lead and do something similar for the quarkus-oidc

@geoand
Copy link
Contributor

geoand commented Jul 1, 2020

@loicmathieu you can now merge this on your own, right?

@loicmathieu
Copy link
Contributor Author

@loicmathieu you can now merge this on your own, right?

Yes, this will be the first time I hit the merge button on a Quarkus PR 🚀

@gsmet
Copy link
Member

gsmet commented Jul 1, 2020

Please when doing substantial doc changes, run the doc build before creating the PR. This one broke the build.

Thanks!

@gsmet
Copy link
Member

gsmet commented Jul 1, 2020

Fixed in #10413

@geoand
Copy link
Contributor

geoand commented Jul 1, 2020

Thanks @gsmet!

@gsmet gsmet added this to the 1.6.0.Final milestone Jul 1, 2020
@loicmathieu
Copy link
Contributor Author

Sorry for this @gsmet, I only run doc build in case of new doc, or when adding links to check them.
I should run the build each time ...

Maybe we can add a doc CI workflow, it should be easy to create one and avoid such issue.

@loicmathieu loicmathieu deleted the doc/oauth2-mock-authorization-server branch August 12, 2020 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Explain how to configure embedded authentication for integration testing

4 participants