From 403b22ade09b3f60dfd1549a446525fc22027eef Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Mon, 9 Sep 2019 19:34:09 -0700 Subject: [PATCH] Working CI Build This change automatically turns off tests where we haven't had a chance to implement wiremocking. They can still be run locally by setting test.github.useProxy (even though most of them do actually use the proxy). --- .github/workflows/maven-build.yml | 2 +- .../org/kohsuke/github/GitHubBuilder.java | 6 +- .../github/AbstractGitHubApiTestBase.java | 26 ++------ .../github/AbstractGitHubApiWireMockTest.java | 59 +++++++++++++++---- .../org/kohsuke/github/GHLicenseTest.java | 10 +--- .../kohsuke/github/GHOrganizationTest.java | 10 +++- .../java/org/kohsuke/github/GitHubTest.java | 3 +- .../org/kohsuke/github/PullRequestTest.java | 7 ++- .../kohsuke/github/RepositoryTrafficTest.java | 16 +++-- 9 files changed, 81 insertions(+), 58 deletions(-) diff --git a/.github/workflows/maven-build.yml b/.github/workflows/maven-build.yml index 0043ce7fe7..28d096e0b4 100644 --- a/.github/workflows/maven-build.yml +++ b/.github/workflows/maven-build.yml @@ -17,4 +17,4 @@ jobs: - name: Maven Download all dependencies run: mvn -B org.apache.maven.plugins:maven-dependency-plugin:3.1.1:go-offline - name: Maven Build - run: mvn -B package --file pom.xml -Dtest=CommitTest,GistTest,PullRequestTest,UserTest,WireMockStatusReporterTest + run: mvn -B package --file pom.xml diff --git a/src/main/java/org/kohsuke/github/GitHubBuilder.java b/src/main/java/org/kohsuke/github/GitHubBuilder.java index 56a784d7ff..a2be04ec04 100644 --- a/src/main/java/org/kohsuke/github/GitHubBuilder.java +++ b/src/main/java/org/kohsuke/github/GitHubBuilder.java @@ -42,13 +42,13 @@ public GitHubBuilder() { * * If there is still no user it means there are no credentials defined and throw an IOException. * - * @return the configured Builder from credentials defined on the system or in the environment. + * @return the configured Builder from credentials defined on the system or in the environment. Otherwise returns null. * * @throws IOException If there are no credentials defined in the ~/.github properties file or the process environment. */ - public static GitHubBuilder fromCredentials() throws IOException { + static GitHubBuilder fromCredentials() throws IOException { Exception cause = null; - GitHubBuilder builder; + GitHubBuilder builder = null; try { builder = fromPropertyFile(); diff --git a/src/test/java/org/kohsuke/github/AbstractGitHubApiTestBase.java b/src/test/java/org/kohsuke/github/AbstractGitHubApiTestBase.java index f246838de6..e35144341b 100644 --- a/src/test/java/org/kohsuke/github/AbstractGitHubApiTestBase.java +++ b/src/test/java/org/kohsuke/github/AbstractGitHubApiTestBase.java @@ -11,32 +11,16 @@ import java.io.File; import java.io.IOException; +import static org.junit.Assume.assumeTrue; + /** * @author Kohsuke Kawaguchi */ -public abstract class AbstractGitHubApiTestBase extends Assert { - - protected GitHub gitHub; - +public abstract class AbstractGitHubApiTestBase extends AbstractGitHubApiWireMockTest { @Before public void setUp() throws Exception { - File f = new File(System.getProperty("user.home"), ".github.kohsuke2"); - if (f.exists()) { - Properties props = new Properties(); - FileInputStream in = null; - try { - in = new FileInputStream(f); - props.load(in); - } finally { - IOUtils.closeQuietly(in); - } - // use the non-standard credential preferentially, so that developers of this library do not have - // to clutter their event stream. - gitHub = GitHubBuilder.fromProperties(props).withRateLimitHandler(RateLimitHandler.FAIL).build(); - } else { - gitHub = GitHubBuilder.fromCredentials().withRateLimitHandler(RateLimitHandler.FAIL).build(); - } + assumeTrue( "All tests inheriting from this class are not guaranteed to work without proxy", useProxy); } protected GHUser getUser() { @@ -49,7 +33,7 @@ protected GHUser getUser() { protected void kohsuke() { String login = getUser().getLogin(); - Assume.assumeTrue(login.equals("kohsuke") || login.equals("kohsuke2")); + assumeTrue(login.equals("kohsuke") || login.equals("kohsuke2")); } protected static final RandomNameGenerator rnd = new RandomNameGenerator(); diff --git a/src/test/java/org/kohsuke/github/AbstractGitHubApiWireMockTest.java b/src/test/java/org/kohsuke/github/AbstractGitHubApiWireMockTest.java index 90739071d9..1d33512751 100644 --- a/src/test/java/org/kohsuke/github/AbstractGitHubApiWireMockTest.java +++ b/src/test/java/org/kohsuke/github/AbstractGitHubApiWireMockTest.java @@ -6,12 +6,16 @@ import com.github.tomakehurst.wiremock.extension.ResponseTransformer; import com.github.tomakehurst.wiremock.http.Request; import com.github.tomakehurst.wiremock.http.Response; +import org.apache.commons.io.IOUtils; import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.kohsuke.github.junit.WireMockRule; +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; import java.util.Properties; import static com.github.tomakehurst.wiremock.client.WireMock.*; @@ -21,12 +25,13 @@ */ public abstract class AbstractGitHubApiWireMockTest extends Assert { - // By default the wiremock tests will, run without proxy or taking a snapshot. + // 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"; @@ -67,25 +72,52 @@ public String getName() { }) ); - @Before - public void wireMockSetup() throws Exception { + private static GitHubBuilder createGitHubBuilder() { - GitHubBuilder builder = GitHubBuilder.fromEnvironment() - .withEndpoint("http://localhost:" + githubApi.port()) - .withRateLimitHandler(RateLimitHandler.FAIL); + GitHubBuilder builder = new GitHubBuilder(); + + try { + File f = new File(System.getProperty("user.home"), ".github.kohsuke2"); + if (f.exists()) { + Properties props = new Properties(); + FileInputStream in = null; + try { + in = new FileInputStream(f); + props.load(in); + } finally { + IOUtils.closeQuietly(in); + } + // use the non-standard credential preferentially, so that developers of this library do not have + // to clutter their event stream. + builder = GitHubBuilder.fromProperties(props); + } else { + builder = GitHubBuilder.fromCredentials(); + } + } 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); + } + + protected GitHubBuilder getGitHubBuilder() { + return githubBuilder; + } + @Before + public void wireMockSetup() throws Exception { if(useProxy) { githubApi.stubFor( proxyAllTo("https://api.github.com/") .atPriority(100) ); } else { - // 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); - // Just to be super clear githubApi.stubFor( any(urlPathMatching(".*")) @@ -93,7 +125,10 @@ public void wireMockSetup() throws Exception { .atPriority(100)); } - gitHub = builder.build(); + + gitHub = getGitHubBuilder() + .withEndpoint("http://localhost:" + githubApi.port()) + .build(); } @After diff --git a/src/test/java/org/kohsuke/github/GHLicenseTest.java b/src/test/java/org/kohsuke/github/GHLicenseTest.java index b2cc73f928..fc039dc45c 100644 --- a/src/test/java/org/kohsuke/github/GHLicenseTest.java +++ b/src/test/java/org/kohsuke/github/GHLicenseTest.java @@ -35,15 +35,7 @@ /** * @author Duncan Dickinson */ -public class GHLicenseTest extends Assert { - private GitHub gitHub; - - @Before - public void setUp() throws Exception { - gitHub = new GitHubBuilder() - .fromCredentials() - .build(); - } +public class GHLicenseTest extends AbstractGitHubApiTestBase { /** * Basic test to ensure that the list of licenses from {@link GitHub#listLicenses()} is returned diff --git a/src/test/java/org/kohsuke/github/GHOrganizationTest.java b/src/test/java/org/kohsuke/github/GHOrganizationTest.java index a1c5d3b921..d724f2450e 100644 --- a/src/test/java/org/kohsuke/github/GHOrganizationTest.java +++ b/src/test/java/org/kohsuke/github/GHOrganizationTest.java @@ -14,7 +14,9 @@ public class GHOrganizationTest extends AbstractGitHubApiTestBase { @Override public void setUp() throws Exception { super.setUp(); - org = gitHub.getOrganization("github-api-test-org"); + if (useProxy) { + org = gitHub.getOrganization("github-api-test-org"); + } } @Test @@ -37,7 +39,9 @@ public void testCreateRepositoryWithAutoInitialization() throws IOException { @After public void cleanUp() throws Exception { - GHRepository repository = org.getRepository(GITHUB_API_TEST); - repository.delete(); + if (useProxy) { + GHRepository repository = org.getRepository(GITHUB_API_TEST); + repository.delete(); + } } } diff --git a/src/test/java/org/kohsuke/github/GitHubTest.java b/src/test/java/org/kohsuke/github/GitHubTest.java index f4b77b8271..962cf9d663 100644 --- a/src/test/java/org/kohsuke/github/GitHubTest.java +++ b/src/test/java/org/kohsuke/github/GitHubTest.java @@ -24,7 +24,8 @@ /** * Unit test for {@link GitHub}. */ -public class GitHubTest { +public class GitHubTest extends AbstractGitHubApiTestBase { + @Test public void testOffline() throws Exception { GitHub hub = GitHub.offline(); diff --git a/src/test/java/org/kohsuke/github/PullRequestTest.java b/src/test/java/org/kohsuke/github/PullRequestTest.java index 7bd0f9f030..5573dc2fa1 100644 --- a/src/test/java/org/kohsuke/github/PullRequestTest.java +++ b/src/test/java/org/kohsuke/github/PullRequestTest.java @@ -222,8 +222,11 @@ public void getUser() throws IOException { @After public void cleanUp() throws Exception { - for (GHPullRequest pr : getRepository().getPullRequests(GHIssueState.OPEN)) { - pr.close(); + // Cleanup is only needed when proxying + if (useProxy) { + for (GHPullRequest pr : getRepository().getPullRequests(GHIssueState.OPEN)) { + pr.close(); + } } } diff --git a/src/test/java/org/kohsuke/github/RepositoryTrafficTest.java b/src/test/java/org/kohsuke/github/RepositoryTrafficTest.java index b1f13e6c97..3ecc6d5897 100644 --- a/src/test/java/org/kohsuke/github/RepositoryTrafficTest.java +++ b/src/test/java/org/kohsuke/github/RepositoryTrafficTest.java @@ -17,9 +17,15 @@ import java.util.List; import java.util.TimeZone; -public class RepositoryTrafficTest { +public class RepositoryTrafficTest extends AbstractGitHubApiTestBase { final private String login = "kohsuke", repositoryName = "github-api"; + @Override + protected GitHubBuilder getGitHubBuilder() { + return new GitHubBuilder() + .withPassword(login, null); + } + @SuppressWarnings("unchecked") private void checkResponse(T expected, T actual){ Assert.assertEquals(expected.getCount(), actual.getCount()); @@ -49,8 +55,6 @@ private void testTraffic(T expectedResult) throw ObjectMapper mapper = new ObjectMapper().setDateFormat(dateFormat); String mockedResponse = mapper.writeValueAsString(expectedResult); - - GitHub gitHub = GitHub.connect(login, null); GitHub gitHubSpy = Mockito.spy(gitHub); GHRepository repo = gitHubSpy.getUser(login).getRepository(repositoryName); @@ -71,8 +75,8 @@ private void testTraffic(T expectedResult) throw // this covers calls on "uc" in Requester.setupConnection and Requester.buildRequest - URL trafficURL = new URL( - "https://api.github.com/repos/"+login+"/"+repositoryName+"/traffic/" + + URL trafficURL = gitHub.getApiURL( + "/repos/"+login+"/"+repositoryName+"/traffic/" + ((expectedResult instanceof GHRepositoryViewTraffic) ? "views" : "clones") ); Mockito.doReturn(mockHttpURLConnection).when(connectorSpy).connect(Mockito.eq(trafficURL)); @@ -149,7 +153,7 @@ public void testGetClones() throws IOException{ @Test public void testGetTrafficStatsAccessFailureDueToInsufficientPermissions() throws IOException { String errorMsg = "Exception should be thrown, since we don't have permission to access repo traffic info."; - GitHub gitHub = GitHub.connect(login, null); + GHRepository repo = gitHub.getUser(login).getRepository(repositoryName); try { repo.getViewTraffic();