-
Notifications
You must be signed in to change notification settings - Fork 127
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
Add way to record and mock http apis #39
Conversation
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
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.
I think you can make this easier to use
* previously recorded mocks before generating new ones is recommended. | ||
*/ | ||
public class WireMockRuleFactory { | ||
private String prop = System.getProperty("toRecord"); |
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.
given that this property is completely under your own control, I would have this at least include wiremock, e.g. wiremock.record
and I would use the same property name both for the system property as for the pom property
//needed for WireMockRule file location | ||
private String mappingLocation = "src/test/resources"; | ||
|
||
public WireMockRecorderRule(Options options, String url){ |
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 seem that the logical way to do this is have the wiremock.record
property value be the URL to proxy... if the property is blank then no recording... if the property is non-blank, set up the recorder for the specified url. That makes running easier...
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.
That's a good point. This would also allow you to potentially record a couple different servers over several different iterations of your tests.
Also fix the location of the Factory file.
Thanks for the review @stephenc I've addressed your comments in a followup PR. They do make it easier to use this feature. |
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.
🐝
public class WireMockRuleFactory { | ||
private String urlToMock = System.getProperty("wiremock.record"); | ||
|
||
public WireMockRule getRule(int port){ |
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.
public WireMockRule getRule(int port) {
At least, we should use the same rules in the file.
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.
I'm not sure what you mean here? Are you talking about @Rule
or including all the potential implementations of WireMockRule? The most common "short" form of creating a rule is by specifying a port, and for extreme flexibility, the Options constructor is included here.
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.
@kwhetstone space between (int port)
and {
in order to apply the same rules in the file
//needed for WireMockRule file location | ||
private String mappingLocation = "src/test/resources"; | ||
|
||
public WireMockRecorderRule(Options options, String url){ |
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.
Ditto
@reviewbybees done. If no one objects, this can be merged. |
I presume that this was designed to simplify jenkinsci/github-branch-source-plugin#79, right? If so, can we see that PR updated to use a |
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.
🐝
* | ||
* Point your system to "http://localhost:8089/" to see mocked responses. | ||
* | ||
* To record mocks: {@code mvn test -wiremock.record="http://api_url"} Clearing |
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.
Did you forget the D
in -D
?
@jglick yes, it is. |
<dependency> | ||
<groupId>com.google.guava</groupId> | ||
<artifactId>guava</artifactId> | ||
<version>19.0</version> |
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.
You do not want to do this. It will clash with the Guava build in Jenkins core.
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.
I need minimally guava-18.0 and the version we're pulling in through Jenkins core is guava-11.0.1. Not 100% sure how to work around this at the moment; I might have to rethink this.
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.
Removed; using wiremock-standalone which will include all the pieces without conflicts.
🐛 with the guava version conflict |
looks to have all dependencies shaded |
public WireMockRecorderRule(Options options, String url) { | ||
super(options); | ||
this.stubFor(get(urlMatching(".*")).atPriority(10).willReturn(aResponse().proxiedFrom(url))); | ||
this.enableRecordMappings(new SingleRootFileSource(mappingLocation + "/mappings"), new SingleRootFileSource(mappingLocation + "/__files")); |
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.
This likely will fail for multi-module projects where new File(".")
is not the same directory as pom.xml
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.
Sure, that could be added as a new option.
Gets around the guice dependency issues.
I've updated the list to cover choosing a custom folder location. The mocks can be recorded from the command line as before. Since recording and mocking will read out of the same folder, no extra configuration is needed there. |
} | ||
|
||
private void finshWireMockRuleSetup(WireMockConfiguration options, String url, String mappingLocation) { | ||
this.stubFor(get(urlMatching(".*")).atPriority(10).willReturn(aResponse().proxiedFrom(url))); |
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.
Spelling of "Finish"
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.
Fixed!
I've made the requested changes. Thanks Jesse and Stephen for the feedback. |
return new WireMockRecorderRule(options, urlToMock, mapLoc); | ||
} else { | ||
return new WireMockRule(options | ||
.usingFilesUnderClasspath(mapLoc)); |
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.
This will not work the way you think if mapLoc is src/test/resources/mockLoc
as you provide in the example javadoc
IIUC with jenkinsci/github-api-plugin#13 the intent is to move away from WireMock. |
Added support for recording & mocking http responses with Wiremock. This particular rule allows a test to serve up prerendered files which correspond to particular api responses. If a response is not saved (trying to access an unmocked url), the test will fail. You can enable a "recording" mode in which you can run your test through on a live server and save the responses for later offline testing. The recordings are saved to
/src/test/resources/
of whatever project uses this rule.While the particular constructors I've included are the ones that have been the most useful to me, they too can be expanded. Other options for the server can be found on wiremock.org
@reviewbybees