websocket test refactor with Junit and arquillian

[gurunrao]

Websocket Platform TCK and Standalone TCK test refactor using Junit and arquillian.

Standalone Test results:

The CI run for the refactored can be found at: https://ci.eclipse.org/jakartaee-tck/job/guru/job/jakarta-websocket-tck-glassfish/20/consoleFull

The test logs can be found at: https://ci.eclipse.org/jakartaee-tck/job/guru/job/jakarta-websocket-tck-glassfish/20/artifact/glassfish-runner/websocket-tck/target/failsafe-reports/

Glassfish logs at: https://ci.eclipse.org/jakartaee-tck/job/guru/job/jakarta-websocket-tck-glassfish/20/artifact/glassfish-runner/websocket-tck/target/glassfish7/glassfish/domains/domain1/logs/server.log

Test run summary:
Tests run: 736, Failures: 0, Errors: 0, Skipped: 0

Platform Test results:

The CI run for the refactored can be found at: https://ci.eclipse.org/jakartaee-tck/job/guru/job/jakarta-websocket-platform-tck-glassfish/2/consoleFull

The test logs can be found at: https://ci.eclipse.org/jakartaee-tck/job/guru/job/jakarta-websocket-platform-tck-glassfish/2/artifact/glassfish-runner/websocket-platform-tck/target/failsafe-reports/

Glassfish logs at: https://ci.eclipse.org/jakartaee-tck/job/guru/job/jakarta-websocket-tck-glassfish/2/artifact/glassfish-runner/websocket-platform-tck/target/glassfish7/glassfish/domains/domain1/logs/server.log

Test run summary:
Tests run: 748, Failures: 0, Errors: 0, Skipped: 0

[olamy]

is it using arquilian?

[gurunrao]

is it using arquilian?

Yes, it uses arquillian and junit5.

[olamy]

is it using arquilian?

Yes, it uses arquillian and junit5.

great. I will have a try next week with Jetty.

[olamy]

Please have a look at this PR https://github.com/gurunrao/jakartaee-tck/pull/8
The host:port should be coming from Arquillian. Some container such jetty just start some random so we cannot expect to have hardcoded port before starting the build

[olamy]

why renaming all classes to *IT?

[gurunrao]

why renaming all classes to *IT?

fail-safe plugin default inclusion wildcards are as follows:
https://maven.apache.org/surefire/maven-failsafe-plugin/examples/inclusion-exclusion.html

[gurunrao]

Please have a look at this PR gurunrao#8 The host:port should be coming from Arquillian. Some container such jetty just start some random so we cannot expect to have hardcoded port before starting the build

The host:port were configured in jtest and harness TCK (https://github.com/jakartaee/platform-tck/blob/master/install/websocket/bin/ts.jte#L177), we intend to keep the functionality as before.

[olamy]

Please have a look at this PR gurunrao#8 The host:port should be coming from Arquillian. Some container such jetty just start some random so we cannot expect to have hardcoded port before starting the build

The host:port were configured in jtest and harness TCK (https://github.com/jakartaee/platform-tck/blob/master/install/websocket/bin/ts.jte#L177), we intend to keep the functionality as before.

Yes but now with Arquillian this works totally different and the change from the PR must be applied. I have tested with gf runner and this works. This has been done as this with servlet tck as this is simply using the hardcoded port return by Arquillian GH container.

If not changed there jetty or any arquillian container using random port will not being able to use this TCK code.

cc @starksm64

the code is part of old tck.

[joakime]

The host:port were configured in jtest and harness TCK (https://github.com/jakartaee/platform-tck/blob/master/install/websocket/bin/ts.jte#L177), we intend to keep the functionality as before.

I don't like this.
The hardcoded port should not be the default or mandatory behavior.
All of the bad behaviors of jtest should be considered deprecated legacy behaviors.
Use junit and arquillian properly and don't hang onto the bad decisions from the past.

[gurunrao]

The host:port were configured in jtest and harness TCK (https://github.com/jakartaee/platform-tck/blob/master/install/websocket/bin/ts.jte#L177), we intend to keep the functionality as before.

I don't like this. The hardcoded port should not be the default or mandatory behavior. All of the bad behaviors of jtest should be considered deprecated legacy behaviors. Use junit and arquillian properly and don't hang onto the bad decisions from the past.

Port is configured using properties file and not hardcoded.

[olamy]

The host:port were configured in jtest and harness TCK (https://github.com/jakartaee/platform-tck/blob/master/install/websocket/bin/ts.jte#L177), we intend to keep the functionality as before.

I don't like this. The hardcoded port should not be the default or mandatory behavior. All of the bad behaviors of jtest should be considered deprecated legacy behaviors. Use junit and arquillian properly and don't hang onto the bad decisions from the past.

Port is configured using properties file and not harcoded.

No, it is definitely hardcoded in the pom.
The port will be defined by Arquillian (e.g each Arquillian impl will have its own file and so it will be provided by the URL injected by Arqullian).
Please look at how every other TCKs (servlet and others) have been translated to Arquillian. They are all using this feature.
If you want to use hardcoded port fair enough but use them only in the glassfish runner part as I'm not if the arquillian glassfush can use random port.
Please look at this PR https://github.com/gurunrao/jakartaee-tck/pull/8. the change is very simple.
especillay this line https://github.com/gurunrao/jakartaee-tck/pull/8/files#diff-cbb04ed2421f4d932ce2e779fd6fb96ebaa87f01886848dbfb4c0fb36d13e946R123

cc @starksm64 @scottmarlow

[gurunrao]

The host:port were configured in jtest and harness TCK (https://github.com/jakartaee/platform-tck/blob/master/install/websocket/bin/ts.jte#L177), we intend to keep the functionality as before.

I don't like this. The hardcoded port should not be the default or mandatory behavior. All of the bad behaviors of jtest should be considered deprecated legacy behaviors. Use junit and arquillian properly and don't hang onto the bad decisions from the past.

Port is configured using properties file and not harcoded.

No, it is definitely hardcoded in the pom. The port will be defined by Arquillian (e.g each Arquillian impl will have its own file and so it will be provided by the URL injected by Arqullian). Please look at how every other TCKs (servlet and others) have been translated to Arquillian. They are all using this feature. If you want to use hardcoded port fair enough but use them only in the glassfish runner part as I'm not if the arquillian glassfush can use random port. Please look at this PR gurunrao#8. the change is very simple. especillay this line https://github.com/gurunrao/jakartaee-tck/pull/8/files#diff-cbb04ed2421f4d932ce2e779fd6fb96ebaa87f01886848dbfb4c0fb36d13e946R123

cc @starksm64 @scottmarlow

https://github.com/gurunrao/jakartaee-tck/pull/8 - doesn't take care of secure ports

[joakime]

gurunrao#8 - doesn't take care of secure ports

The example of gurunrao#8 shows how it works with a default deployment.

	@ArquillianResource
	@OperateOnDeployment("_DEFAULT_")
	public URL url;

Take the Servlet world, for example, if the need is for an Arquillian operation with a secure port, then it is accessed with a operational name.

platform-tck/servlet/src/main/java/com/sun/ts/tests/servlet/spec/security/clientcertanno/Client.java

Lines 63 to 64 in 5d9911b

	@ArquillianResource
	@OperateOnDeployment("webapp-https") URL urlHttps;

and used like ...

platform-tck/servlet/src/main/java/com/sun/ts/tests/servlet/spec/security/clientcertanno/Client.java

Lines 112 to 113 in 5d9911b

	portnum = urlHttps.getPort();
	p.setProperty("securedWebServicePort", Integer.toString(portnum));

Which would be needed in only one websocket test class ...
websocket/src/main/java/com/sun/ts/tests/websocket/platform/jakarta/websocket/server/handshakerequest/authenticatedssl/WSCClientIT.java

[markt-asf]

Some feedback from testing this with Tomcat.

First the big plus in the improved speed. Tests execute much faster with the entire suite complete in just over 3 minutes.

It took a little while to figure out how to configure the TCK to run with Tomcat but now I have figured that it it will be easy to maintain going forward and to potentially feed into CI systems to run the TCK more regularly.

I am currently seeing a small number (three) of test failures. I need to look into these to figure out if it is my configuration of the test suite that is still not quite right or if something else is going on.

[markt-asf]

There does appear to be a concurrency issue with the refactored tests.

A number of the tests define private static StringBuffer receivedMessageString which is then used to log messages at key points thoughout the test which are then validated at the end of the test. The Maven Failsafe plugin runs all the tests in a single JVM by default and that appeared to be causing issues with the validation of receivedMessageString.

I'm not sure what the "right" fix is but configuring the tests to run with <reuseForks>false</reuseForks> fixed those failures. The downside is that the tests run in just over 5 minutes rather than just over 3 minutes (which is still much faster than the old TCK). The "right" fix might be breaking up those tests into separate classes each with their own private static StringBuffer receivedMessageString so they don't interfere.

I'm still seeing a couple of timeout tests fail but at the moment I think this is a Tomcat configuration issue.

[markt-asf]

It isn't a Tomcat issue. The root cause is the same as the other failures. The problem is that setTimeout2Test sees the message TCKTestServer second message after sleep generated by setTimeout1Test and fails. My quick fix was to use unique messages for the two timeout tests.

That has resolved the failures I was seeing but it looks like one of the timeout tests is now failing further on. I'm still investigating.

[markt-asf]

Woot! The remaining issues were configuration (I'd forgotten a system property needed to ensure the timeouts fire promptly enough for the tests) and a Tomact bug caused by a recent refactoring. All 736 tests are now passing which is consistent with the old TCK tests that had 737 tests (including the signature test which hasn't been refactored yet).

[olamy]

@gurunrao please read my comment here #1182 (comment)
maybe we can have something which could make everybody happy?

[gurunrao]

gurunrao#8 - doesn't take care of secure ports

The example of gurunrao#8 shows how it works with a default deployment. Please refer latest code.

	@ArquillianResource
	@OperateOnDeployment("_DEFAULT_")
	public URL url;

Take the Servlet world, for example, if the need is for an Arquillian operation with a secure port, then it is accessed with a operational name.

platform-tck/servlet/src/main/java/com/sun/ts/tests/servlet/spec/security/clientcertanno/Client.java

Lines 63 to 64 in 5d9911b

	@ArquillianResource
	@OperateOnDeployment("webapp-https") URL urlHttps;

and used like ...

platform-tck/servlet/src/main/java/com/sun/ts/tests/servlet/spec/security/clientcertanno/Client.java

Lines 112 to 113 in 5d9911b

	portnum = urlHttps.getPort();
	p.setProperty("securedWebServicePort", Integer.toString(portnum));

Which would be needed in only one websocket test class ... websocket/src/main/java/com/sun/ts/tests/websocket/platform/jakarta/websocket/server/handshakerequest/authenticatedssl/WSCClientIT.java

I have made necessary changes as per comments, the ssl port auto-detection is not getting detected.

[gurunrao]

gurunrao#8 - doesn't take care of secure ports

The example of gurunrao#8 shows how it works with a default deployment. Please refer latest code.

	@ArquillianResource
	@OperateOnDeployment("_DEFAULT_")
	public URL url;

Take the Servlet world, for example, if the need is for an Arquillian operation with a secure port, then it is accessed with a operational name.

platform-tck/servlet/src/main/java/com/sun/ts/tests/servlet/spec/security/clientcertanno/Client.java

Lines 63 to 64 in 5d9911b

	@ArquillianResource
	@OperateOnDeployment("webapp-https") URL urlHttps;

and used like ...

platform-tck/servlet/src/main/java/com/sun/ts/tests/servlet/spec/security/clientcertanno/Client.java

Lines 112 to 113 in 5d9911b

	portnum = urlHttps.getPort();
	p.setProperty("securedWebServicePort", Integer.toString(portnum));

Which would be needed in only one websocket test class ... websocket/src/main/java/com/sun/ts/tests/websocket/platform/jakarta/websocket/server/handshakerequest/authenticatedssl/WSCClientIT.java

I have made necessary changes as per comments, the ssl port auto-detection is not getting detected.

@olamy @joakime any suggestions on SSL port auto-detection working?

[olamy]

@gurunrao I'm on holidays. I can have a look next week.
This is how we do for Jetty https://github.com/jetty-project/servlet-tck-run/blob/jetty-12-ee10/src/test/resources/arquillian.xml

as you can see there is the default

<container qualifier="http" default="true">

and then the https one

<container qualifier="https">

https here it's just an id in the arquillian configuration.

And you reference this as is in your test.

platform-tck/servlet/src/main/java/com/sun/ts/tests/servlet/spec/security/clientcert/Client.java

Line 73 in 644743f

@TargetsContainer("https")

[scottmarlow]

We should consider asking on Websocket dev mailing list if the Standalone WebSocket TCK tests should move to the WebSocket spec. I see that Mark Thomas is here helping already but its not too early to discuss moving the WebSocket TCK work to the Specification.

We started that conversation recently on the Servlet mailing list.

[gurunrao]

@gurunrao I'm on holidays. I can have a look next week. This is how we do for Jetty https://github.com/jetty-project/servlet-tck-run/blob/jetty-12-ee10/src/test/resources/arquillian.xml

as you can see there is the default

<container qualifier="http" default="true">

and then the https one

<container qualifier="https">

https here it's just an id in the arquillian configuration.

And you reference this as is in your test.

platform-tck/servlet/src/main/java/com/sun/ts/tests/servlet/spec/security/clientcert/Client.java

Line 73 in 644743f

@TargetsContainer("https")

https://github.com/gurunrao/jakartaee-tck/blob/tck-refactor-websocket-2/glassfish-runner/websocket-platform-tck/src/test/resources/arquillian.xml#L10 has details on container.

[gurunrao]

if there are no new review comments,I am planning to merge the PR.

[gurunrao]

@gurunrao I'm on holidays. I can have a look next week. This is how we do for Jetty https://github.com/jetty-project/servlet-tck-run/blob/jetty-12-ee10/src/test/resources/arquillian.xml
as you can see there is the default

<container qualifier="http" default="true">

and then the https one

<container qualifier="https">

https here it's just an id in the arquillian configuration.
And you reference this as is in your test.

platform-tck/servlet/src/main/java/com/sun/ts/tests/servlet/spec/security/clientcert/Client.java

Line 73 in 644743f

@TargetsContainer("https")

https://github.com/gurunrao/jakartaee-tck/blob/tck-refactor-websocket-2/glassfish-runner/websocket-platform-tck/src/test/resources/arquillian.xml#L10 has details on container.

I will not debug the auto detection of SSL further, Since we have system properties and @olamy suggested SSL port auto-detection might be issue with glassfish arquillian.