diff --git a/pom.xml b/pom.xml index 8b69185653..90009735b1 100644 --- a/pom.xml +++ b/pom.xml @@ -56,11 +56,11 @@ org.codehaus.mojo animal-sniffer-maven-plugin - 1.15 + 1.18 org.codehaus.mojo.signature - java15 + java17 1.0 diff --git a/src/test/java/org/kohsuke/github/AbstractGitHubApiTestBase.java b/src/test/java/org/kohsuke/github/AbstractGitHubApiTestBase.java index e35144341b..c0cd49e312 100644 --- a/src/test/java/org/kohsuke/github/AbstractGitHubApiTestBase.java +++ b/src/test/java/org/kohsuke/github/AbstractGitHubApiTestBase.java @@ -20,7 +20,7 @@ public abstract class AbstractGitHubApiTestBase extends AbstractGitHubApiWireMoc @Before public void setUp() throws Exception { - assumeTrue( "All tests inheriting from this class are not guaranteed to work without proxy", useProxy); + assumeTrue( "All tests inheriting from this class are not guaranteed to work without proxy", githubApi.isUseProxy()); } protected GHUser getUser() { diff --git a/src/test/java/org/kohsuke/github/AbstractGitHubApiWireMockTest.java b/src/test/java/org/kohsuke/github/AbstractGitHubApiWireMockTest.java index 1d33512751..91e20e9044 100644 --- a/src/test/java/org/kohsuke/github/AbstractGitHubApiWireMockTest.java +++ b/src/test/java/org/kohsuke/github/AbstractGitHubApiWireMockTest.java @@ -11,6 +11,8 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Rule; +import org.junit.rules.TestWatcher; +import org.kohsuke.github.junit.GitHubApiWireMockRule; import org.kohsuke.github.junit.WireMockRule; import java.io.File; @@ -21,55 +23,36 @@ import static com.github.tomakehurst.wiremock.client.WireMock.*; /** - * @author Kohsuke Kawaguchi + * @author Liam Newman */ public abstract class AbstractGitHubApiWireMockTest extends Assert { - // By default the wiremock tests will run without proxy or taking a snapshot. - // The tests will use only the stubbed data and will fail if requests are made for missing data. - // You can use the proxy without taking a snapshot while writing and debugging tests. - // You cannot take a snapshot without proxying. - protected final static boolean takeSnapshot = System.getProperty("test.github.takeSnapshot", "false") != "false"; - protected final static boolean useProxy = takeSnapshot || System.getProperty("test.github.useProxy", "false") != "false"; private final GitHubBuilder githubBuilder = createGitHubBuilder(); public final static String STUBBED_USER_LOGIN = "placeholder-user"; public final static String STUBBED_USER_PASSWORD = "placeholder-password"; + /** + * {@link GitHub} instance for use during test. + * Traffic will be part of snapshot when taken. + */ protected GitHub gitHub; - private final String baseFilesClassPath = this.getClass().getName().replace('.', '/'); - protected final String baseRecordPath = "src/test/resources/" + baseFilesClassPath + "/wiremock"; + /** + * {@link GitHub} instance for use before/after test. + * Traffic will not be part of snapshot when taken. + * Should only be used when isUseProxy() or isTakeSnapShot(). + */ + protected GitHub gitHubBeforeAfter; + protected final String baseFilesClassPath = this.getClass().getName().replace('.', '/'); + protected final String baseRecordPath = "src/test/resources/" + baseFilesClassPath + "/wiremock"; @Rule - public WireMockRule githubApi = new WireMockRule(WireMockConfiguration.options() - .dynamicPort() - .usingFilesUnderDirectory(baseRecordPath) - .extensions( - new ResponseTransformer() { - @Override - public Response transform(Request request, Response response, FileSource files, - Parameters parameters) { - if ("application/json" - .equals(response.getHeaders().getContentTypeHeader().mimeTypePart()) - && !response.getHeaders().getHeader("Content-Encoding").containsValue("gzip")) { - return Response.Builder.like(response) - .but() - .body(response.getBodyAsString() - .replace("https://api.github.com/", - "http://localhost:" + githubApi.port() + "/") - ) - .build(); - } - return response; - } - - @Override - public String getName() { - return "github-api-url-rewrite"; - } - }) + public GitHubApiWireMockRule githubApi = new GitHubApiWireMockRule( + WireMockConfiguration.options() + .dynamicPort() + .usingFilesUnderDirectory(baseRecordPath) ); private static GitHubBuilder createGitHubBuilder() { @@ -96,13 +79,6 @@ private static GitHubBuilder createGitHubBuilder() { } catch (IOException e) { } - if (!useProxy) { - // This sets the user and password to a placeholder for wiremock testing - // This makes the tests believe they are running with permissions - // The recorded stubs will behave like they running with permissions - builder.withPassword(STUBBED_USER_LOGIN, STUBBED_USER_PASSWORD); - } - return builder.withRateLimitHandler(RateLimitHandler.FAIL); } @@ -112,31 +88,25 @@ protected GitHubBuilder getGitHubBuilder() { @Before public void wireMockSetup() throws Exception { - if(useProxy) { - githubApi.stubFor( - proxyAllTo("https://api.github.com/") - .atPriority(100) - ); - } else { - // Just to be super clear - githubApi.stubFor( - any(urlPathMatching(".*")) - .willReturn(status(500).withBody("Stubbed data not found. Set test.github.use-proxy to have WireMock proxy to github")) - .atPriority(100)); - } + GitHubBuilder builder = getGitHubBuilder(); + if (!githubApi.isUseProxy()) { + // This sets the user and password to a placeholder for wiremock testing + // This makes the tests believe they are running with permissions + // The recorded stubs will behave like they running with permissions + builder.withPassword(STUBBED_USER_LOGIN, STUBBED_USER_PASSWORD); + } - gitHub = getGitHubBuilder() + gitHub = builder .withEndpoint("http://localhost:" + githubApi.port()) .build(); - } - @After - public void snapshotRequests() { - if (takeSnapshot) { - githubApi.snapshotRecord(recordSpec() - .forTarget("https://api.github.com") - .extractTextBodiesOver(255)); + if (githubApi.isUseProxy()) { + gitHubBeforeAfter = builder + .withEndpoint("https://api.github.com/") + .build(); + } else { + gitHubBeforeAfter = null; } } } diff --git a/src/test/java/org/kohsuke/github/GHOrganizationTest.java b/src/test/java/org/kohsuke/github/GHOrganizationTest.java index d724f2450e..ca32bf72cd 100644 --- a/src/test/java/org/kohsuke/github/GHOrganizationTest.java +++ b/src/test/java/org/kohsuke/github/GHOrganizationTest.java @@ -14,7 +14,7 @@ public class GHOrganizationTest extends AbstractGitHubApiTestBase { @Override public void setUp() throws Exception { super.setUp(); - if (useProxy) { + if (githubApi.isUseProxy()) { org = gitHub.getOrganization("github-api-test-org"); } } @@ -39,9 +39,9 @@ public void testCreateRepositoryWithAutoInitialization() throws IOException { @After public void cleanUp() throws Exception { - if (useProxy) { - GHRepository repository = org.getRepository(GITHUB_API_TEST); - repository.delete(); - } + if (githubApi.isUseProxy()) { + GHRepository repository = org.getRepository(GITHUB_API_TEST); + repository.delete(); } } +} diff --git a/src/test/java/org/kohsuke/github/PullRequestTest.java b/src/test/java/org/kohsuke/github/PullRequestTest.java index 5573dc2fa1..f6c78fc679 100644 --- a/src/test/java/org/kohsuke/github/PullRequestTest.java +++ b/src/test/java/org/kohsuke/github/PullRequestTest.java @@ -15,6 +15,17 @@ */ public class PullRequestTest extends AbstractGitHubApiWireMockTest { + @Before + @After + public void cleanUp() throws Exception { + // Cleanup is only needed when proxying + if (!githubApi.isUseProxy()) return; + + for (GHPullRequest pr : getRepository(this.gitHubBeforeAfter).getPullRequests(GHIssueState.OPEN)) { + pr.close(); + } + } + @Test public void createPullRequest() throws Exception { String name = "createPullRequest"; @@ -220,17 +231,11 @@ public void getUser() throws IOException { } } - @After - public void cleanUp() throws Exception { - // Cleanup is only needed when proxying - if (useProxy) { - for (GHPullRequest pr : getRepository().getPullRequests(GHIssueState.OPEN)) { - pr.close(); - } - } + protected GHRepository getRepository() throws IOException { + return getRepository(gitHub); } - private GHRepository getRepository() throws IOException { + private GHRepository getRepository(GitHub gitHub) throws IOException { return gitHub.getOrganization("github-api-test-org").getRepository("github-api"); } } diff --git a/src/test/java/org/kohsuke/github/WireMockStatusReporterTest.java b/src/test/java/org/kohsuke/github/WireMockStatusReporterTest.java index 717dfe1d08..a93fef8277 100644 --- a/src/test/java/org/kohsuke/github/WireMockStatusReporterTest.java +++ b/src/test/java/org/kohsuke/github/WireMockStatusReporterTest.java @@ -20,8 +20,8 @@ public class WireMockStatusReporterTest extends AbstractGitHubApiWireMockTest { @Test public void user_whenProxying_AuthCorrectlyConfigured() throws Exception { - assumeFalse("Test only valid when not taking a snapshot", takeSnapshot); - assumeTrue("Test only valid when proxying (-Dtest.github.useProxy to enable)", useProxy); + assumeFalse("Test only valid when not taking a snapshot", githubApi.isTakeSnapshot()); + assumeTrue("Test only valid when proxying (-Dtest.github.useProxy to enable)", githubApi.isUseProxy()); assertThat( "GitHub connection believes it is anonymous. Make sure you set GITHUB_OAUTH or both GITHUB_USER and GITHUB_PASSWORD environment variables", @@ -42,8 +42,8 @@ public void user_whenProxying_AuthCorrectlyConfigured() throws Exception { @Test public void user_whenNotProxying_Stubbed() throws Exception { - assumeFalse("Test only valid when not taking a snapshot", takeSnapshot); - assumeFalse("Test only valid when not proxying", useProxy); + assumeFalse("Test only valid when not taking a snapshot", githubApi.isTakeSnapshot()); + assumeFalse("Test only valid when not proxying", githubApi.isUseProxy()); assertThat(gitHub.isAnonymous(), is(false)); assertThat(gitHub.login, equalTo(STUBBED_USER_LOGIN)); @@ -59,8 +59,8 @@ public void user_whenNotProxying_Stubbed() throws Exception { @Test public void BasicBehaviors_whenNotProxying() throws Exception { - assumeFalse("Test only valid when not taking a snapshot", takeSnapshot); - assumeFalse("Test only valid when not proxying", useProxy); + assumeFalse("Test only valid when not taking a snapshot", githubApi.isTakeSnapshot()); + assumeFalse("Test only valid when not proxying", githubApi.isUseProxy()); Exception e = null; GHRepository repo = null; @@ -96,8 +96,8 @@ public void BasicBehaviors_whenNotProxying() throws Exception { @Test public void BasicBehaviors_whenProxying() throws Exception { - assumeFalse("Test only valid when not taking a snapshot", takeSnapshot); - assumeTrue("Test only valid when proxying (-Dtest.github.useProxy to enable)", useProxy); + assumeFalse("Test only valid when not taking a snapshot", githubApi.isTakeSnapshot()); + assumeTrue("Test only valid when proxying (-Dtest.github.useProxy to enable)", githubApi.isUseProxy()); Exception e = null; GHRepository repo = null; @@ -123,15 +123,15 @@ public void BasicBehaviors_whenProxying() throws Exception { @Test public void whenSnapshot_EnsureProxy() throws Exception { - assumeTrue("Test only valid when Snapshotting (-Dtest.github.takeSnapshot to enable)", takeSnapshot); + assumeTrue("Test only valid when Snapshotting (-Dtest.github.takeSnapshot to enable)", githubApi.isTakeSnapshot()); - assertTrue("When taking a snapshot, proxy should automatically be enabled", useProxy); + assertTrue("When taking a snapshot, proxy should automatically be enabled", githubApi.isUseProxy()); } @Ignore("Not implemented yet") @Test public void whenSnapshot_EnsureRecordToExpectedLocation() throws Exception { - assumeTrue("Test only valid when Snapshotting (-Dtest.github.takeSnapshot to enable)", takeSnapshot); + assumeTrue("Test only valid when Snapshotting (-Dtest.github.takeSnapshot to enable)", githubApi.isTakeSnapshot()); } } diff --git a/src/test/java/org/kohsuke/github/junit/GitHubApiWireMockRule.java b/src/test/java/org/kohsuke/github/junit/GitHubApiWireMockRule.java new file mode 100644 index 0000000000..eac1654c15 --- /dev/null +++ b/src/test/java/org/kohsuke/github/junit/GitHubApiWireMockRule.java @@ -0,0 +1,94 @@ +package org.kohsuke.github.junit; + +import com.github.tomakehurst.wiremock.common.FileSource; +import com.github.tomakehurst.wiremock.core.Options; +import com.github.tomakehurst.wiremock.core.WireMockConfiguration; +import com.github.tomakehurst.wiremock.extension.Parameters; +import com.github.tomakehurst.wiremock.extension.ResponseTransformer; +import com.github.tomakehurst.wiremock.http.Request; +import com.github.tomakehurst.wiremock.http.Response; + +import static com.github.tomakehurst.wiremock.client.WireMock.*; +import static com.github.tomakehurst.wiremock.client.WireMock.status; + +/** + * @author Liam Newman + */ +public class GitHubApiWireMockRule extends WireMockRule { + + // By default the wiremock tests will run without proxy or taking a snapshot. + // The tests will use only the stubbed data and will fail if requests are made for missing data. + // You can use the proxy without taking a snapshot while writing and debugging tests. + // You cannot take a snapshot without proxying. + private final static boolean takeSnapshot = System.getProperty("test.github.takeSnapshot", "false") != "false"; + private final static boolean useProxy = takeSnapshot || System.getProperty("test.github.useProxy", "false") != "false"; + + public GitHubApiWireMockRule(WireMockConfiguration options) { + this(options, true); + } + + public GitHubApiWireMockRule(WireMockConfiguration options, boolean failOnUnmatchedRequests) { + super(options + .extensions( + new ResponseTransformer() { + @Override + public Response transform(Request request, Response response, FileSource files, + Parameters parameters) { + if ("application/json" + .equals(response.getHeaders().getContentTypeHeader().mimeTypePart()) + && !response.getHeaders().getHeader("Content-Encoding").containsValue("gzip")) { + return Response.Builder.like(response) + .but() + .body(response.getBodyAsString() + .replace("https://api.github.com/", + "http://localhost:" + request.getPort() + "/") + ) + .build(); + } + return response; + } + + @Override + public String getName() { + return "github-api-url-rewrite"; + } + }), + failOnUnmatchedRequests); + } + + public boolean isUseProxy() { + return GitHubApiWireMockRule.useProxy; + } + + public boolean isTakeSnapshot() { + return GitHubApiWireMockRule.takeSnapshot; + } + + @Override + protected void before() { + super.before(); + if(isUseProxy()) { + this.stubFor( + proxyAllTo("https://api.github.com/") + .atPriority(100) + ); + } else { + // Just to be super clear + this.stubFor( + any(urlPathMatching(".*")) + .willReturn(status(500).withBody("Stubbed data not found. Set test.github.use-proxy to have WireMock proxy to github")) + .atPriority(100)); + } + } + + @Override + protected void after() { + super.after(); + if (isTakeSnapshot()) { + this.snapshotRecord(recordSpec() + .forTarget("https://api.github.com") + .captureHeader("If-None-Match") + .extractTextBodiesOver(255)); + } + } +} diff --git a/src/test/java/org/kohsuke/github/junit/WireMockRule.java b/src/test/java/org/kohsuke/github/junit/WireMockRule.java index e0e5d09a43..dd16fa112a 100644 --- a/src/test/java/org/kohsuke/github/junit/WireMockRule.java +++ b/src/test/java/org/kohsuke/github/junit/WireMockRule.java @@ -45,12 +45,21 @@ import org.junit.runners.model.Statement; import wiremock.com.google.common.base.Optional; +/** + * @author Liam Newman + */ public class WireMockRule implements MethodRule, TestRule, Container, Stubbing, Admin { private WireMockServer wireMockServer; private boolean failOnUnmatchedRequests; private final Options options; + public String getMethodName() { + return methodName; + } + + private String methodName = null; + public WireMockRule(Options options) { this(options, true); } @@ -83,6 +92,7 @@ public Statement apply(final Statement base, FrameworkMethod method, Object targ private Statement apply(final Statement base, final String methodName) { return new Statement() { public void evaluate() throws Throwable { + WireMockRule.this.methodName = methodName; final Options localOptions = new WireMockOptions( WireMockRule.this.options, methodName); @@ -101,6 +111,7 @@ public void evaluate() throws Throwable { } finally { WireMockRule.this.after(); WireMockRule.this.stop(); + WireMockRule.this.methodName = null; } }