Issue #5798 fix jetty runner startup error and add jsp support as well #6115
Conversation
Signed-off-by: olivier lamy <[email protected]>
Signed-off-by: olivier lamy <[email protected]>
Signed-off-by: olivier lamy <[email protected]>
| </plugin> | ||
| </plugins> | ||
| </build> | ||
| </project> |
There was a problem hiding this comment.
@olamy would any of this test setup be simplified at all if it moved over into tests/test-runner instead? The integration test setup is impressive, but looks really complex.
There was a problem hiding this comment.
could be but it will be simply the same poms etc... move to tests/test-runner.
What if we want to add more tests? I don't want to turn having tests/test-runner-2 etc...
There was a problem hiding this comment.
This uses the jetty-runner.jar as built, and without the maven classloader causing false success.
Does tests/test-runner satisfy these 2 (kinda important) aspects of testing jetty runner?
There was a problem hiding this comment.
will only move this IT test as a simple project under tests/test-runner. But I don't like it. I prefer this structure using the invoker plugin within jetty-runner project.
jetty-runner/src/main/java/org/eclipse/jetty/runner/Runner.java
Outdated
Show resolved
Hide resolved
Signed-off-by: olivier lamy <[email protected]>
|
@joakime nudge |
| @@ -164,6 +170,7 @@ public void usage(String error) | |||
| System.err.println(" --out file - info/warn/debug log filename (with optional 'yyyy_mm_dd' wildcard"); | |||
| System.err.println(" --host name|ip - interface to listen on (default is all interfaces)"); | |||
| System.err.println(" --port n - port to listen on (default 8080)"); | |||
| System.err.println(" --port-file path - file to write a single line with the port used to serve http requests "); | |||
There was a problem hiding this comment.
This should probably be more than just the port.
It should likely contain the value of Server.getURI() (after Server.start())so that the scheme + network address + port + context path are all communicated.
There was a problem hiding this comment.
Would --server-uri-file path be a better option?
Signed-off-by: olivier lamy <[email protected]>
| HttpClient httpClient = new HttpClient(); | ||
| try | ||
| { | ||
| httpClient.start(); | ||
| ContentResponse response = | ||
| httpClient.newRequest("localhost", port).path("/").send(); | ||
| ContentResponse response = httpClient.newRequest(serverUri).send(); |
There was a problem hiding this comment.
Keep in mind that the serverUri will contain the contextPath of the first ContextHandler, if it isn't / then you'll need to URI.create(serverUri).resolve("/") to use the path at root.
There was a problem hiding this comment.
but here we have only one war deployed.
There was a problem hiding this comment.
interesting the documentation says [[--path /path] context]*n but looking at the code we do not support this n ;)
No description provided.