RFR: 8264190: Harden TLS interop tests

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8264190: Harden TLS interop tests

Fernando Guallini-2
Occasional interop tests failures may occur when making use of the test/jdk/javax/net/ssl/TLSCommon/interop framework since there is no assurance the selected available port it is still free at the time a server using the framework starts up, for instance, by command line. To mitigate intermittent failures, this patch updates interop/BaseInteropTest.java in order to validate the server is alive and if not, retry to create a valid server.

In addition, Utilities::isSessionResumed implementation is now comparing equality of first and second session creation time to validate session resumption

-------------

Commit messages:
 - Harden TLS interop tests

Changes: https://git.openjdk.java.net/jdk/pull/3218/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3218&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264190
  Stats: 28 lines in 4 files changed: 19 ins; 1 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3218.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3218/head:pull/3218

PR: https://git.openjdk.java.net/jdk/pull/3218
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264190: Harden TLS interop tests [v2]

Fernando Guallini-2
> Occasional interop tests failures may occur when making use of the test/jdk/javax/net/ssl/TLSCommon/interop framework since there is no assurance the selected available port it is still free at the time a server using the framework starts up, for instance, by command line. To mitigate intermittent failures, this patch updates interop/BaseInteropTest.java in order to validate the server is alive and if not, retry to create a valid server.
>
> In addition, Utilities::isSessionResumed implementation is now comparing equality of first and second session creation time to validate session resumption

Fernando Guallini has updated the pull request incrementally with one additional commit since the last revision:

  no need to check if alive while iterating during accept

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3218/files
  - new: https://git.openjdk.java.net/jdk/pull/3218/files/e9b984b9..474f482d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3218&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3218&range=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3218.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3218/head:pull/3218

PR: https://git.openjdk.java.net/jdk/pull/3218
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264190: Harden TLS interop tests [v3]

Fernando Guallini-2
In reply to this post by Fernando Guallini-2
> Occasional interop tests failures may occur when making use of the test/jdk/javax/net/ssl/TLSCommon/interop framework since there is no assurance the selected available port it is still free at the time a server using the framework starts up, for instance, by command line. To mitigate intermittent failures, this patch updates interop/BaseInteropTest.java in order to validate the server is alive and if not, retry to create a valid server.
>
> In addition, Utilities::isSessionResumed implementation is now comparing equality of first and second session creation time to validate session resumption

Fernando Guallini has updated the pull request incrementally with one additional commit since the last revision:

  Improve JdkServer::close and Utilities::waitFor methods

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3218/files
  - new: https://git.openjdk.java.net/jdk/pull/3218/files/474f482d..5688014b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3218&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3218&range=01-02

  Stats: 5 lines in 2 files changed: 2 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3218.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3218/head:pull/3218

PR: https://git.openjdk.java.net/jdk/pull/3218
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264190: Harden TLS interop tests [v3]

Fernando Guallini-2
On Mon, 29 Mar 2021 17:31:59 GMT, Fernando Guallini <[hidden email]> wrote:

>> Occasional interop tests failures may occur when making use of the test/jdk/javax/net/ssl/TLSCommon/interop framework since there is no assurance the selected available port it is still free at the time a server using the framework starts up, for instance, by command line. To mitigate intermittent failures, this patch updates interop/BaseInteropTest.java in order to validate the server is alive and if not, retry to create a valid server.
>>
>> In addition, Utilities::isSessionResumed implementation is now comparing equality of first and second session creation time to validate session resumption
>
> Fernando Guallini has updated the pull request incrementally with one additional commit since the last revision:
>
>   Improve JdkServer::close and Utilities::waitFor methods

test/jdk/javax/net/ssl/TLSCommon/interop/JdkServer.java line 145:

> 143:     @Override
> 144:     public void close() throws IOException {
> 145:         if (!serverSocket.isClosed()) {

isAlive can be overridden with a more complex implementation, whereas the only default requirement to close the socket in this case is checking it is not already closed.

test/jdk/javax/net/ssl/TLSCommon/interop/Utilities.java line 220:

> 218:      *  Sleeps until the condition is true or getting timeout.
> 219:      */
> 220:     public static <T> boolean waitFor(Predicate<T> predicate, T t) {

waitFor method was evaluating predicate at least twice with previous implementation even when true

-------------

PR: https://git.openjdk.java.net/jdk/pull/3218
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264190: Harden TLS interop tests [v3]

Rajan Halade-2
In reply to this post by Fernando Guallini-2
On Mon, 29 Mar 2021 17:31:59 GMT, Fernando Guallini <[hidden email]> wrote:

>> Occasional interop tests failures may occur when making use of the test/jdk/javax/net/ssl/TLSCommon/interop framework since there is no assurance the selected available port it is still free at the time a server using the framework starts up, for instance, by command line. To mitigate intermittent failures, this patch updates interop/BaseInteropTest.java in order to validate the server is alive and if not, retry to create a valid server.
>>
>> In addition, Utilities::isSessionResumed implementation is now comparing equality of first and second session creation time to validate session resumption
>
> Fernando Guallini has updated the pull request incrementally with one additional commit since the last revision:
>
>   Improve JdkServer::close and Utilities::waitFor methods

test/jdk/javax/net/ssl/TLSCommon/interop/BaseInteropTest.java line 236:

> 234:             // Retry operation, server might have failed to bind a port
> 235:             server.signalStop();
> 236:             server = createServer(useCase, executor);

Can you please define MAX_RETRIES and use loop to retry? This will make it flexible for us to update retries if needed and avoid duplicate code.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3218
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264190: Harden TLS interop tests [v4]

Fernando Guallini-2
In reply to this post by Fernando Guallini-2
> Occasional interop tests failures may occur when making use of the test/jdk/javax/net/ssl/TLSCommon/interop framework since there is no assurance the selected available port it is still free at the time a server using the framework starts up, for instance, by command line. To mitigate intermittent failures, this patch updates interop/BaseInteropTest.java in order to validate the server is alive and if not, retry to create a valid server.
>
> In addition, Utilities::isSessionResumed implementation is now comparing equality of first and second session creation time to validate session resumption

Fernando Guallini has updated the pull request incrementally with one additional commit since the last revision:

  add max_retries count, move shell methods to ProcUtils and add win methods

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3218/files
  - new: https://git.openjdk.java.net/jdk/pull/3218/files/5688014b..b29e4886

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3218&range=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3218&range=02-03

  Stats: 143 lines in 3 files changed: 93 ins; 42 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3218.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3218/head:pull/3218

PR: https://git.openjdk.java.net/jdk/pull/3218
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264190: Harden TLS interop tests [v3]

Fernando Guallini-2
In reply to this post by Rajan Halade-2
On Mon, 29 Mar 2021 18:45:47 GMT, Rajan Halade <[hidden email]> wrote:

>> Fernando Guallini has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Improve JdkServer::close and Utilities::waitFor methods
>
> test/jdk/javax/net/ssl/TLSCommon/interop/BaseInteropTest.java line 236:
>
>> 234:             // Retry operation, server might have failed to bind a port
>> 235:             server.signalStop();
>> 236:             server = createServer(useCase, executor);
>
> Can you please define MAX_RETRIES and use loop to retry? This will make it flexible for us to update retries if needed and avoid duplicate code.

Done, also retries value can be now overridden by subclasses if we wanted. In addition, moved Utilities::shell and shellProc to ProcUtils class which is a better location and added support for running win commands

-------------

PR: https://git.openjdk.java.net/jdk/pull/3218
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264190: Harden TLS interop tests [v4]

Rajan Halade-2
In reply to this post by Fernando Guallini-2
On Wed, 31 Mar 2021 10:53:34 GMT, Fernando Guallini <[hidden email]> wrote:

>> Occasional interop tests failures may occur when making use of the test/jdk/javax/net/ssl/TLSCommon/interop framework since there is no assurance the selected available port it is still free at the time a server using the framework starts up, for instance, by command line. To mitigate intermittent failures, this patch updates interop/BaseInteropTest.java in order to validate the server is alive and if not, retry to create a valid server.
>>
>> In addition, Utilities::isSessionResumed implementation is now comparing equality of first and second session creation time to validate session resumption
>
> Fernando Guallini has updated the pull request incrementally with one additional commit since the last revision:
>
>   add max_retries count, move shell methods to ProcUtils and add win methods

Marked as reviewed by rhalade (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/3218