From 8abb52d3699aea75d99931c1722df3c75e863080 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Fri, 19 Nov 2021 12:42:31 -0800 Subject: [PATCH 1/4] Revert "Workaround limitation of jacoco" This reverts commit a0136af17e62251d0e96820edd05a6552482da8c. --- pom.xml | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/pom.xml b/pom.xml index 0e37999aad..078eaaf91e 100644 --- a/pom.xml +++ b/pom.xml @@ -800,9 +800,49 @@ + + maven-antrun-plugin + 3.0.0 + + + + process-test-classes + + + run + + + + + + + + + + + + + + + + + maven-surefire-plugin + + java11-test + test + + test + + + ${project.basedir}/target/classes-test-java11 + false + src/test/resources/slow-or-flaky-tests.txt + @{jacoco.surefire.argLine} ${surefire.argLine} -Dtest.github.connector=httpclient + + java11-jar-test integration-test From 777c36162db5082fec9129f51806eb2555b4b01a Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Fri, 19 Nov 2021 14:38:57 -0800 Subject: [PATCH 2/4] Revert "Move back to Java 8 for code coverage" This reverts commit 5699e5899d331f4d69aa70c4fd6e8aefcf8aa6c3. --- .github/workflows/maven-build.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/maven-build.yml b/.github/workflows/maven-build.yml index 64ffe29cd6..cd86e8a4ca 100644 --- a/.github/workflows/maven-build.yml +++ b/.github/workflows/maven-build.yml @@ -94,8 +94,13 @@ jobs: if: matrix.os != 'windows' && startsWith(matrix.java, '8') uses: codecov/codecov-action@v2.1.0 # JDK 11+ - - name: Maven Install without Code Coverage (Java 11+) - if: (!startsWith(matrix.java, '8')) + - name: Maven Install without Code Coverage + if: matrix.os == 'windows' && !startsWith(matrix.java, '8') env: MAVEN_OPTS: ${{ env.JAVA_11_PLUS_MAVEN_OPTS }} run: mvn -B clean install --file pom.xml "-Dsurefire.argLine=--add-opens java.base/java.net=ALL-UNNAMED" + - name: Maven Install with Code Coverage + if: matrix.os != 'windows' && !startsWith(matrix.java, '8') + env: + MAVEN_OPTS: ${{ env.JAVA_11_PLUS_MAVEN_OPTS }} + run: mvn -B clean install -D enable-ci --file pom.xml "-Dsurefire.argLine=--add-opens java.base/java.net=ALL-UNNAMED" From 6834d3a6a0b78044614576e0e8a41fd7bc7a6953 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Fri, 19 Nov 2021 14:40:23 -0800 Subject: [PATCH 3/4] Enable code coverage on Java 11 The resulting ouput reports coverage numbers for HttpClientGitHubConnector, but points to the wrong file. Bummer, but at least the numbers are right. --- pom.xml | 127 ++++++++++++++++++++------------------------------------ 1 file changed, 45 insertions(+), 82 deletions(-) diff --git a/pom.xml b/pom.xml index 078eaaf91e..17db303236 100644 --- a/pom.xml +++ b/pom.xml @@ -103,40 +103,36 @@ org.jacoco jacoco-maven-plugin 0.8.7 + + + + /org/kohsuke/github/extras/HttpClient* + + - prepare-agent + prepare-agent-integration jacoco.surefire.argLine - - - org.kohsuke.* - org/kohsuke/* - - - org/kohsuke/github/extras/HttpClientGitHubConnector* - org/kohsuke/github/extras/HttpClientGitHubConnector*.* - META-INF/versions/11/org/kohsuke/github/extras/HttpClientGitHubConnector*.* - report - test - report + report-integration check - test + verify check + ${project.build.directory}/jacoco-it.exec BUNDLE @@ -231,6 +227,27 @@ true + + org.codehaus.mojo + animal-sniffer-maven-plugin + 1.20 + + + org.codehaus.mojo.signature + java18 + 1.0 + + + + + ensure-java-1.8-class-library + test + + check + + + + @@ -296,28 +313,6 @@ org.codehaus.mojo animal-sniffer-maven-plugin - 1.20 - - - org.codehaus.mojo.signature - java18 - 1.0 - - - - - java.net.http.* - - - - - ensure-java-1.8-class-library - test - - check - - - com.infradna.tool @@ -592,22 +587,24 @@ okhttp-test - test + integration-test test + ${project.basedir}/target/github-api-${project.version}.jar src/test/resources/slow-or-flaky-tests.txt @{jacoco.surefire.argLine} ${surefire.argLine} -Dtest.github.connector=okhttp slow-or-flaky-test - test + integration-test test + ${project.basedir}/target/github-api-${project.version}.jar 2 @@ -677,7 +674,7 @@ - ${project.build.directory}/jacoco.exec + ${project.build.directory}/jacoco-it.exec @@ -747,12 +744,18 @@ multirelease [11,) - - !test - + + org.codehaus.mojo + animal-sniffer-maven-plugin + + + java.net.http.* + + + maven-compiler-plugin 3.8.1 @@ -800,51 +803,11 @@ - - maven-antrun-plugin - 3.0.0 - - - - process-test-classes - - - run - - - - - - - - - - - - - - - - - maven-surefire-plugin java11-test - test - - test - - - ${project.basedir}/target/classes-test-java11 - false - src/test/resources/slow-or-flaky-tests.txt - @{jacoco.surefire.argLine} ${surefire.argLine} -Dtest.github.connector=httpclient - - - - java11-jar-test integration-test test From 027d19af05f25d3a11d353dfd180b2831b709a4d Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Fri, 19 Nov 2021 17:00:28 -0800 Subject: [PATCH 4/4] Code clean up --- .../connector/GitHubConnectorResponse.java | 22 ---------- .../kohsuke/github/AbuseLimitHandlerTest.java | 44 ++++++++++--------- 2 files changed, 24 insertions(+), 42 deletions(-) diff --git a/src/main/java/org/kohsuke/github/connector/GitHubConnectorResponse.java b/src/main/java/org/kohsuke/github/connector/GitHubConnectorResponse.java index 75595727ee..1790a183d3 100644 --- a/src/main/java/org/kohsuke/github/connector/GitHubConnectorResponse.java +++ b/src/main/java/org/kohsuke/github/connector/GitHubConnectorResponse.java @@ -112,26 +112,4 @@ public int statusCode() { public Map> allHeaders() { return headers; } - - /** - * Unwraps a {@link GitHubConnectorResponse} from a {@link HttpURLConnection} adapter. - * - * Only works on the internal {@link GitHubConnectorResponseHttpUrlConnectionAdapter}. - * - * @param connection - * the connection to unwrap. - * @return an unwrapped response from an adapter. - * @throws UnsupportedOperationException - * if the connection is not an adapter. - * @deprecated Only preset for testing and interaction with deprecated HttpURLConnection components. - */ - @Deprecated - public final static GitHubConnectorResponse fromHttpURLConnectionAdapter(HttpURLConnection connection) { - if (connection instanceof GitHubConnectorResponseHttpUrlConnectionAdapter) { - return ((GitHubConnectorResponseHttpUrlConnectionAdapter) connection).connectorResponse(); - } else { - throw new UnsupportedOperationException( - "Cannot unwrap GitHubConnectorResponse from " + connection.getClass().getName()); - } - } } diff --git a/src/test/java/org/kohsuke/github/AbuseLimitHandlerTest.java b/src/test/java/org/kohsuke/github/AbuseLimitHandlerTest.java index c09d15d18d..d496cc9aea 100644 --- a/src/test/java/org/kohsuke/github/AbuseLimitHandlerTest.java +++ b/src/test/java/org/kohsuke/github/AbuseLimitHandlerTest.java @@ -5,7 +5,6 @@ import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; -import org.kohsuke.github.connector.GitHubConnectorResponse; import java.io.IOException; import java.io.InputStream; @@ -60,13 +59,6 @@ public void testHandler_Fail() throws Exception { @Override public void onError(IOException e, HttpURLConnection uc) throws IOException { - GitHubConnectorResponse connectorResponse = null; - try { - connectorResponse = GitHubConnectorResponse.fromHttpURLConnectionAdapter(uc); - } catch (UnsupportedOperationException ex) { - assertThat(ex.getMessage(), startsWith("Cannot unwrap GitHubConnectorResponse")); - } - // Verify assertThat(uc.getDate(), Matchers.greaterThanOrEqualTo(new Date().getTime() - 10000)); assertThat(uc.getExpiration(), equalTo(0L)); @@ -90,20 +82,34 @@ public void onError(IOException e, HttpURLConnection uc) throws IOException { // getting an input stream in an error case should throw IOException ioEx = Assert.assertThrows(IOException.class, () -> uc.getInputStream()); - InputStream errorStream = uc.getErrorStream(); - assertThat(errorStream, notNullValue()); - String error = IOUtils.toString(errorStream, StandardCharsets.UTF_8); - assertThat(error, containsString("Must have push access to repository")); - - if (connectorResponse != null) { - String connectorBody = IOUtils.toString(connectorResponse.bodyStream(), - StandardCharsets.UTF_8); - assertThat(connectorBody, containsString("Must have push access to repository")); + try (InputStream errorStream = uc.getErrorStream()) { + assertThat(errorStream, notNullValue()); + String errorString = IOUtils.toString(errorStream, StandardCharsets.UTF_8); + assertThat(errorString, containsString("Must have push access to repository")); } // calling again should still error ioEx = Assert.assertThrows(IOException.class, () -> uc.getInputStream()); + // calling again on a GitHubConnectorResponse should yield the same value + if (uc.toString().contains("GitHubConnectorResponseHttpUrlConnectionAdapter")) { + try (InputStream errorStream = uc.getErrorStream()) { + assertThat(errorStream, notNullValue()); + String errorString = IOUtils.toString(errorStream, StandardCharsets.UTF_8); + assertThat(errorString, containsString("Must have push access to repository")); + } + } else { + try (InputStream errorStream = uc.getErrorStream()) { + assertThat(errorStream, notNullValue()); + String errorString = IOUtils.toString(errorStream, StandardCharsets.UTF_8); + fail(); + assertThat(errorString, containsString("Must have push access to repository")); + } catch (IOException ex) { + assertThat(ex, notNullValue()); + assertThat(ex.getMessage(), containsString("stream is closed")); + } + } + assertThat(uc.getHeaderFields(), instanceOf(Map.class)); assertThat(uc.getHeaderFields().size(), Matchers.greaterThan(25)); assertThat(uc.getHeaderField("Status"), equalTo("403 Forbidden")); @@ -130,9 +136,7 @@ public void onError(IOException e, HttpURLConnection uc) throws IOException { Assert.assertThrows(IllegalStateException.class, () -> uc.setRequestProperty("bogus", "thing")); Assert.assertThrows(IllegalStateException.class, () -> uc.setUseCaches(true)); - if (connectorResponse != null) { - assertThat(uc.toString(), - containsString("GitHubConnectorResponseHttpUrlConnectionAdapter")); + if (uc.toString().contains("GitHubConnectorResponseHttpUrlConnectionAdapter")) { Assert.assertThrows(UnsupportedOperationException.class, () -> uc.getAllowUserInteraction());