-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[fix][test] Replace calls to Auth0 with calls to wiremock #20500
Conversation
Codecov Report
@@ Coverage Diff @@
## master #20500 +/- ##
=============================================
+ Coverage 36.78% 72.98% +36.19%
- Complexity 12059 31941 +19882
=============================================
Files 1690 1867 +177
Lines 129001 138575 +9574
Branches 14041 15213 +1172
=============================================
+ Hits 47453 101133 +53680
+ Misses 75294 29424 -45870
- Partials 6254 8018 +1764
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, good work!
/** | ||
* Mock OIDC (and therefore OAuth2) server for testing. Note that the client_id is mapped to the token's subject claim. | ||
*/ | ||
public class MockOIDCIdentityProvider { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be useful to make this implement AutoCloseable so that it can be directly used with Java's try with resources. This would require that the stop method is renamed to "close".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we want to re-create the wiremock server for each test? Or are you thinking of using an annotation to close it when the object is no longer in scope?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we want to re-create the wiremock server for each test? Or are you thinking of using an annotation to close it when the object is no longer in scope?
No. It was a generic comment about resource classes. Supporting AutoCloseable helps since you could use it with try-with-resources if there would be a need to have instances for a short scope. It's not necessary to make this change for this PR.
(cherry picked from commit da04f24)
Fixes #20473
Motivation
Our tests should be independent of external services like Auth0. We can use
WireMock
to create a server that responds to all OAuth2 requests. This PR removes calls to Auth0.com and replaces them with calls to a wiremock server.Modifications
Verifying this change
Tests are modified and still pass.
Documentation
doc-not-needed