You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by GitBox <gi...@apache.org> on 2020/08/05 12:29:38 UTC

[GitHub] [httpcomponents-core] michael-o opened a new pull request #212: Enable IT for H2 Protocol Negotiation

michael-o opened a new pull request #212:
URL: https://github.com/apache/httpcomponents-core/pull/212


   Since TLS 1.3 and/or APLN is now available in Java 8 we can enable this and avoid the failure on Java 8.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o commented on pull request #212: Enable IT for H2 Protocol Negotiation

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #212:
URL: https://github.com/apache/httpcomponents-core/pull/212#issuecomment-669194004


   Closin per discussion.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o commented on a change in pull request #212: Enable IT for H2 Protocol Negotiation

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #212:
URL: https://github.com/apache/httpcomponents-core/pull/212#discussion_r465706546



##########
File path: httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/H2ProtocolNegotiationTest.java
##########
@@ -233,7 +233,7 @@ public void testNegotiateProtocol() throws Exception {
         final HttpResponse response1 = message1.getHead();
         Assert.assertThat(response1.getCode(), CoreMatchers.equalTo(HttpStatus.SC_OK));
 
-        if (javaVersion >= 9) {
+        if (javaVersion >= 8) {

Review comment:
       Ancient on Trusty: java version "1.8.0_151". This one is almost 4 years old. I consider this to be a realistic test env anymore.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o commented on a change in pull request #212: Enable IT for H2 Protocol Negotiation

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #212:
URL: https://github.com/apache/httpcomponents-core/pull/212#discussion_r465710354



##########
File path: httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/H2ProtocolNegotiationTest.java
##########
@@ -233,7 +233,7 @@ public void testNegotiateProtocol() throws Exception {
         final HttpResponse response1 = message1.getHead();
         Assert.assertThat(response1.getCode(), CoreMatchers.equalTo(HttpStatus.SC_OK));
 
-        if (javaVersion >= 9) {
+        if (javaVersion >= 8) {

Review comment:
       We don't have code which exposes the patch version. Since neither Jekins not Travis have recent JDKs installed, I don't see a way to move this forward :-(




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o commented on a change in pull request #212: Enable IT for H2 Protocol Negotiation

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #212:
URL: https://github.com/apache/httpcomponents-core/pull/212#discussion_r465706546



##########
File path: httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/H2ProtocolNegotiationTest.java
##########
@@ -233,7 +233,7 @@ public void testNegotiateProtocol() throws Exception {
         final HttpResponse response1 = message1.getHead();
         Assert.assertThat(response1.getCode(), CoreMatchers.equalTo(HttpStatus.SC_OK));
 
-        if (javaVersion >= 9) {
+        if (javaVersion >= 8) {

Review comment:
       Ancient on Trusty: java version "1.8.0_151"




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] carterkozak commented on a change in pull request #212: Enable IT for H2 Protocol Negotiation

Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #212:
URL: https://github.com/apache/httpcomponents-core/pull/212#discussion_r465716546



##########
File path: httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/H2ProtocolNegotiationTest.java
##########
@@ -233,7 +233,7 @@ public void testNegotiateProtocol() throws Exception {
         final HttpResponse response1 = message1.getHead();
         Assert.assertThat(response1.getCode(), CoreMatchers.equalTo(HttpStatus.SC_OK));
 
-        if (javaVersion >= 9) {
+        if (javaVersion >= 8) {

Review comment:
       Looks like the backport was 8u251, not 8u252. off by one error in my previous comment: https://www.oracle.com/java/technologies/javase/8u251-relnotes.html
   
   How about something like this?
   
   ```java
       private boolean isAlpnAvailable() {
           if (javaVersion > 8) {
               return true;
           }
           if (javaVersion == 8) {
               // ALPN was backported to java 8u251. Using a java 8 runtime this can be detected
               // based on the presence of the 'getApplicationProtocol' method.
               try {
                   SSLEngine.class.getMethod("getApplicationProtocol");
                   return true;
               } catch (NoSuchMethodException e) {
                   return false;
               }
           }
           return false;
       }
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] carterkozak commented on a change in pull request #212: Enable IT for H2 Protocol Negotiation

Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #212:
URL: https://github.com/apache/httpcomponents-core/pull/212#discussion_r465701522



##########
File path: httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/H2ProtocolNegotiationTest.java
##########
@@ -233,7 +233,7 @@ public void testNegotiateProtocol() throws Exception {
         final HttpResponse response1 = message1.getHead();
         Assert.assertThat(response1.getCode(), CoreMatchers.equalTo(HttpStatus.SC_OK));
 
-        if (javaVersion >= 9) {
+        if (javaVersion >= 8) {

Review comment:
       I'm not sure what versions the various CI servers use, so it may be helpful to check for 8u252+




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #212: Enable IT for H2 Protocol Negotiation

Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #212:
URL: https://github.com/apache/httpcomponents-core/pull/212#discussion_r465716829



##########
File path: httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/H2ProtocolNegotiationTest.java
##########
@@ -233,7 +233,7 @@ public void testNegotiateProtocol() throws Exception {
         final HttpResponse response1 = message1.getHead();
         Assert.assertThat(response1.getCode(), CoreMatchers.equalTo(HttpStatus.SC_OK));
 
-        if (javaVersion >= 9) {
+        if (javaVersion >= 8) {

Review comment:
       @michael-o This is the reason I have not enabled ALPN tests on Java 1.8. I could not find a reliable way of finding out the exact build number to make the test dependent on it. It is not worth it. Please close the PR.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] carterkozak commented on pull request #212: Enable IT for H2 Protocol Negotiation

Posted by GitBox <gi...@apache.org>.
carterkozak commented on pull request #212:
URL: https://github.com/apache/httpcomponents-core/pull/212#issuecomment-670584448


   What do you think about #215? This test flakes every time I run tests locally, I'd like to either update the test to verify correctness on all java 8 versions, or add an `assumeThat` to ignore the test on java 8.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o closed pull request #212: Enable IT for H2 Protocol Negotiation

Posted by GitBox <gi...@apache.org>.
michael-o closed pull request #212:
URL: https://github.com/apache/httpcomponents-core/pull/212


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o edited a comment on pull request #212: Enable IT for H2 Protocol Negotiation

Posted by GitBox <gi...@apache.org>.
michael-o edited a comment on pull request #212:
URL: https://github.com/apache/httpcomponents-core/pull/212#issuecomment-669194004


   Closing as per discussion.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org