Closed ops4j-issues closed 8 years ago
Marc Schlegel commented
Work-in-progress can be found in fork.
First iteration:
Achim Nierbeck: I would like to make the codebase more readable. But before I start migrating every itest I thought it would be good to get your opinion on this.
I will not merge this without a review once it is done.
Achim Nierbeck commented
Marc Schlegel this looks very elegant and I like it.
The only constraint I have right now, it should also work with undertow and tomcat as Server.
So if you could make sure it also works there as client fine with me to switch to that way of defining tests.
Marc Schlegel commented
I've just commited the migration of the karaf-tests. I will fix a ignored test which does post-requests where I have to extend the test-api.
The only issue I came accross is, that the Jetty HttpClient is more strict regarding authentication-challenges: it will complain about missing WWW-Authenticate header when receiving a 401 as a result to a request without Authenticate header.
I've updated the custom AuthorizationContexts in the sample-modules.
So, calling a server based on Jetty, Undertow or Tomcat should not bother the client.
Marc Schlegel commented
All tests migrated with some minor test-failures
Achim Nierbeck commented
found one reason. The waitForServer fails due to connect exceptions, those aren't handled. I'm taking care of this. Just need a stable connection to push.
Achim Nierbeck commented
this fixes the jetty issues:
https://github.com/ops4j/org.ops4j.pax.web/commit/414e3a3c0e7038c7e5da1ba7d84ddb907d6930bb
Marc Schlegel commented
I also did some further investigation. On my local system only three Karaf-Integrationtests fail (WhiteboardKarafTest), two of those I can fix by wrapping the test-execution within a WaitCondition2 which retries the test for 10 seconds.
To me it seems like the deployment on the actual server is not always finished until the test starts. I would like to check if the webcontainers provide some callbacks or methods to check if the webapplication is really available. Calling the context-root from pax-web would not be a good idea since it cannot distinguish between a unfinished deployment or an error in the deployed application.
Achim Nierbeck commented
With my fixes I'm now down to 2 failing (WhiteboardKarafTest) tests with karaf.
Though it doesn't happen when test is run alone, or within eclipse.
Achim Nierbeck commented
actually running the tests a second time, everything is clean ...
Marc Schlegel commented
That fits my observations as well...
It seems to me that the webcontainer is just not done internally setting up the application(s). Maybe the webcontainers(s) offer some API to check that the deployment has been completed (the WebEvent.DEPLOYED in Pax-Web might be fired to early).
I do not understand why this is not a problem with the old TestClient. The only reason I can think of is that the old impl is gathering some state since the instance is bound to a ITestBase for the whole test.
Since all tests have been abstracted now, it would be possible to bring the Apache HttpComponents TestClient back. Though I would prefer to stick with Jetty since it is much easier to digest whats happening.
Achim Nierbeck commented
don't think we need to switch back.
actually regarding the karaf whiteboard test, this is a race condition of the blueprint configuration. If the Filter is registered beforehand of the servlet this'll happen.
Marc Schlegel commented
I have increased the timeout for the JettyClient to 30 seconds after I figured out that the Apache HttpComponentsblocks indefinately when no explicit timeout has been set.
One can ensure the connection manager does not block indefinitely in the connection request operation by setting 'http.conn-manager.timeout' to a positive value.
This could explain why tests are failing unpreditable in Jenkins with timeouts that are running in current master.
EDIT: well, this didn't work. Even worse, the failed tests have increased by 3. I've resetted the branch to the state before my change
Marc Schlegel commented
Failing tests have been caused by the name used for the jenkins-job. Whitespaces can wreak havoc in some plugins.
Final code-cleanup has been performed, now that all tests are stable with the new API.
Note: once JUnit 5 is out, the custom assertion classes can probably be removed because JUnit 5 will add group-assertions.
Marc Schlegel created PAXWEB-984
The integration-tests could be modernized
[]()Replace Apache HttpComponents with Jetty HttpClient
[]()Abstract URL-testing behind a fluent and descriptive API
[]()Make use of Java 8 features
Affects: 6.0.0 Fixed in: 6.0.0 Votes: 0, Watches: 2
Referenced issues
depends on:
1272 - Jenkins Build not working when using a fresh Jenkins-Workspace [PAXWEB-994]