Issue #8 - alternate /tests/ setup for 100% maven #45
Conversation
+ Deploys jetty9:testing (tagged) image in /jetty9/ module during integration-test phase + Adds /tests/ + Adds /tests/webapps/ + Adds /tests/gcloud-testing-core/ for common testing lib
| </exec> | ||
| <exec executable="gcloud"> | ||
| <arg value="docker"/> | ||
| <arg value="push"/> |
There was a problem hiding this comment.
I think this push to GCR should be moved to a different profile or deploy phase.
There was a problem hiding this comment.
This is important for the jetty9:testing tag, as used by the various docker images int he test webapps.
| import java.util.Map; | ||
| import java.util.Objects; | ||
|
|
||
| public final class AppDeployment { |
There was a problem hiding this comment.
Some javadoc comments please on what this class is doing.
There was a problem hiding this comment.
| import java.nio.charset.Charset; | ||
| import java.nio.charset.StandardCharsets; | ||
|
|
||
| public final class HttpURLUtil { |
There was a problem hiding this comment.
Its specifically designed for the HttpURLConnection from the JDK, so it make sense (and conforms to the Google checkstyle rules)
However, I filed it as Issue #49
| <version>1.8</version> | ||
| <executions> | ||
| <execution> | ||
| <id>find-app-deploy-project</id> |
There was a problem hiding this comment.
This is cool how you inject the current project into Maven properties! 👍
The whole block needs a comment though to make it clear what's going on.
There was a problem hiding this comment.
I'll add the comment.
There was a problem hiding this comment.
| List<Entry> entries = new ArrayList<>(); | ||
| Path logPath = MavenTestingUtils.getTargetPath("remote-module-version.log"); | ||
| try (OutputStream output = Files.newOutputStream(logPath, StandardOpenOption.CREATE)) { | ||
| int retval = ProcessUtil.exec(output, "gcloud", "beta", "logging", "read", filter); |
There was a problem hiding this comment.
Can we use the Logging client library instead of gcloud CLI?
There was a problem hiding this comment.
Filed as separate issue #47
| </configuration> | ||
|
|
||
| </plugin> | ||
| <!-- This can be re-enabled when app-maven-plugin issue is fixed |
There was a problem hiding this comment.
Can this just move into the individual test sub-modules right now? ...Oh, you already did that.
There was a problem hiding this comment.
Done that already. But this is a note (like others) for future refactoring once appengine-maven-plugin features catch up.
| @Test | ||
| public void testGet() throws IOException | ||
| { | ||
| HttpURLConnection http = HttpURLUtil.openTo(AppDeployment.SERVER_URI.resolve("/hello/")); |
There was a problem hiding this comment.
We may need to implement URL fetch with retries because sometimes it takes a few seconds for the app to really be up after it's deployed.
There was a problem hiding this comment.
Opened as issue #48
| @@ -0,0 +1,41 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
What's blocking us from eliminating the run.sh altogether?
There was a problem hiding this comment.
Its been removed in joakime@dd97cc8
|
Will create new issue-8 branch locally to this repo and issue a new pull request |
module during integration-test phase