Make HttpClientGitHubConnector the default for Java 11+

hub4j · Nov 21, 2021 · 6c21554 · 6c21554
1 parent 988f603
commit 6c21554
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 9 deletions.
diff --git a/pom.xml b/pom.xml
@@ -825,6 +825,19 @@
                   <argLine>@{jacoco.surefire.argLine} ${surefire.argLine} -Dtest.github.connector=httpclient</argLine>
                 </configuration>
               </execution>
+              <execution>
+                <id>java11-urlconnection-test</id>
+                <phase>integration-test</phase>
+                <goals>
+                  <goal>test</goal>
+                </goals>
+                <configuration>
+                  <classesDirectory>${project.basedir}/target/github-api-${project.version}.jar</classesDirectory>
+                  <useSystemClassLoader>false</useSystemClassLoader>
+                  <excludesFile>src/test/resources/slow-or-flaky-tests.txt</excludesFile>
+                  <argLine>@{jacoco.surefire.argLine} ${surefire.argLine} -Dtest.github.connector=urlconnection</argLine>
+                </configuration>
+              </execution>
             </executions>
           </plugin>
         </plugins>

diff --git a/src/main/java/org/kohsuke/github/GitHubClient.java b/src/main/java/org/kohsuke/github/GitHubClient.java
@@ -391,7 +391,7 @@ public <T> GitHubResponse<T> sendRequest(GitHubRequest request, @CheckForNull Bo
                     connectorRequest = e.connectorRequest;
                 }
             } catch (SocketException | SocketTimeoutException | SSLHandshakeException e) {
-                // These transient errors the
+                // These transient errors thrown by HttpURLConnection
                 if (retries > 0) {
                     logRetryConnectionError(e, request.url(), retries);
                     continue;

diff --git a/src/main/java/org/kohsuke/github/internal/DefaultGitHubConnector.java b/src/main/java/org/kohsuke/github/internal/DefaultGitHubConnector.java
@@ -50,11 +50,11 @@ static GitHubConnector create(String defaultConnectorProperty) {
         } else if (defaultConnectorProperty.equalsIgnoreCase("httpclient")) {
             return new HttpClientGitHubConnector();
         } else if (defaultConnectorProperty.equalsIgnoreCase("default")) {
-            // try {
-            // return new HttpClientGitHubConnector();
-            // } catch (UnsupportedOperationException | LinkageError e) {
-            return new GitHubConnectorHttpConnectorAdapter(HttpConnector.DEFAULT);
-            // }
+            try {
+                return new HttpClientGitHubConnector();
+            } catch (UnsupportedOperationException | LinkageError e) {
+                return new GitHubConnectorHttpConnectorAdapter(HttpConnector.DEFAULT);
+            }
         } else {
             throw new IllegalStateException(
                     "Property 'test.github.connector' must reference a valid built-in connector - okhttp, okhttpconnector, urlconnection, or default.");

diff --git a/src/test/java/org/kohsuke/github/RequesterRetryTest.java b/src/test/java/org/kohsuke/github/RequesterRetryTest.java
@@ -4,11 +4,14 @@
 import com.github.tomakehurst.wiremock.stubbing.Scenario;
 import okhttp3.ConnectionPool;
 import okhttp3.OkHttpClient;
+import org.junit.Assume;
 import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Test;
+import org.kohsuke.github.extras.HttpClientGitHubConnector;
 import org.kohsuke.github.extras.ImpatientHttpConnector;
 import org.kohsuke.github.extras.okhttp3.OkHttpGitHubConnector;
+import org.kohsuke.github.internal.DefaultGitHubConnector;
 import org.mockito.Mockito;
 
 import java.io.ByteArrayOutputStream;
@@ -78,7 +81,7 @@ public void resetTestCapturedLog() throws IOException {
         attachLogCapturer();
     }
 
-    @Ignore("Used okhttp3 and this to verify connection closing. To variable for CI system.")
+    @Ignore("Used okhttp3 and this to verify connection closing. Too flaky for CI system.")
     @Test
     public void testGitHubIsApiUrlValid() throws Exception {
 
@@ -109,6 +112,9 @@ public void testGitHubIsApiUrlValid() throws Exception {
     // Issue #539
     @Test
     public void testSocketConnectionAndRetry() throws Exception {
+        // Only implemented for HttpURLConnection connectors
+        Assume.assumeThat(DefaultGitHubConnector.create(), not(instanceOf(HttpClientGitHubConnector.class)));
+
         // CONNECTION_RESET_BY_PEER errors result in two requests each
         // to get this failure for "3" tries we have to do 6 queries.
         this.mockGitHub.apiServer()
@@ -136,6 +142,9 @@ public void testSocketConnectionAndRetry() throws Exception {
     // Issue #539
     @Test
     public void testSocketConnectionAndRetry_StatusCode() throws Exception {
+        // Only implemented for HttpURLConnection connectors
+        Assume.assumeThat(DefaultGitHubConnector.create(), not(instanceOf(HttpClientGitHubConnector.class)));
+
         // CONNECTION_RESET_BY_PEER errors result in two requests each
         // to get this failure for "3" tries we have to do 6 queries.
         this.mockGitHub.apiServer()
@@ -164,6 +173,9 @@ public void testSocketConnectionAndRetry_StatusCode() throws Exception {
 
     @Test
     public void testSocketConnectionAndRetry_Success() throws Exception {
+        // Only implemented for HttpURLConnection connectors
+        Assume.assumeThat(DefaultGitHubConnector.create(), not(instanceOf(HttpClientGitHubConnector.class)));
+
         // CONNECTION_RESET_BY_PEER errors result in two requests each
         // to get this failure for "3" tries we have to do 6 queries.
         // If there are only 5 errors we succeed.

diff --git a/src/test/java/org/kohsuke/github/internal/DefaultGitHubConnectorTest.java b/src/test/java/org/kohsuke/github/internal/DefaultGitHubConnectorTest.java
@@ -37,8 +37,6 @@ public void testCreate() throws Exception {
 
         connector = DefaultGitHubConnector.create("default");
 
-        // Current implementation never uses httpclient for default.
-        usingHttpClient = false;
         if (usingHttpClient) {
             assertThat(connector, instanceOf(HttpClientGitHubConnector.class));
         } else {