Merge pull request #540 from bitwiseman/test-run

Working CI Build

.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

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();

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();

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,33 +72,63 @@ 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(".*"))
                     .willReturn(status(500).withBody("Stubbed data not found. Set test.github.use-proxy to have WireMock proxy to github"))
                     .atPriority(100));
         }
 
-        gitHub = builder.build();
+
+        gitHub = getGitHubBuilder()
+            .withEndpoint("http://localhost:" + githubApi.port())
+            .build();
     }
 
     @After

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

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();
+        }
     }
 }

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();

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();
+            }
         }
     }
 

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 <T extends GHRepositoryTraffic> void checkResponse(T expected, T actual){
         Assert.assertEquals(expected.getCount(), actual.getCount());
@@ -49,8 +55,6 @@ private <T extends GHRepositoryTraffic> 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 <T extends GHRepositoryTraffic> 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();