Skip to content

Commit f4fde11

Browse files
committed
Addressed code review comments. Removed testing apps from build by default, as build artifact of each is a fat war, containing all jars from the rest of the client libraries (including tests) and is >20MB each.
PTAL
1 parent 9b5e93d commit f4fde11

File tree

8 files changed

+56
-40
lines changed

8 files changed

+56
-40
lines changed

pom.xml

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -114,17 +114,21 @@
114114
<module>google-cloud-monitoring</module>
115115
<module>google-cloud-pubsub</module>
116116
<module>google-cloud-resourcemanager</module>
117-
<module>google-cloud-spanner</module>
117+
<!--<module>google-cloud-spanner</module>-->
118118
<module>google-cloud-speech</module>
119119
<module>google-cloud-storage</module>
120120
<module>google-cloud-trace</module>
121121
<module>google-cloud-translate</module>
122122
<module>google-cloud-vision</module>
123-
<module>testing/google-cloud-managed-test</module>
124-
<module>testing/google-cloud-appengine-flex-java</module>
125-
<module>testing/google-cloud-appengine-flex-custom</module>
126-
<module>testing/google-cloud-appengine-java8</module>
127-
<module>testing/google-cloud-appengine-flex-compat</module>
123+
<!--
124+
Uncomment the following lines to include google app engine (GAE) testing apps in the build.
125+
Each GAE app packages all its dependencies in a fat war (>20MB each).
126+
-->
127+
<!--<module>testing/google-cloud-managed-test</module>-->
128+
<!--<module>testing/google-cloud-appengine-flex-java</module>-->
129+
<!--<module>testing/google-cloud-appengine-flex-custom</module>-->
130+
<!--<module>testing/google-cloud-appengine-java8</module>-->
131+
<!--<module>testing/google-cloud-appengine-flex-compat</module>-->
128132
</modules>
129133
<pluginRepositories>
130134
<pluginRepository>
@@ -214,7 +218,7 @@
214218
<executions>
215219
<execution>
216220
<goals>
217-
<goal>integration-test</goal>
221+
<!--<goal>integration-test</goal>-->
218222
<goal>verify</goal>
219223
</goals>
220224
</execution>

testing/google-cloud-appengine-flex-compat/pom.xml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
<properties>
1717
<appengine.sdk.version>1.9.51</appengine.sdk.version>
18+
<appengine.maven.plugin>1.2.1</appengine.maven.plugin>
1819
<jetty.maven.plugin>9.3.8.v20160314</jetty.maven.plugin>
1920
<java.source.version>1.7</java.source.version>
2021
</properties>
@@ -54,9 +55,12 @@
5455
</configuration>
5556
</plugin>
5657
<plugin>
57-
<groupId>com.google.appengine</groupId>
58+
<groupId>com.google.cloud.tools</groupId>
5859
<artifactId>appengine-maven-plugin</artifactId>
59-
<version>${appengine.sdk.version}</version>
60+
<version>${appengine.maven.plugin}</version>
61+
<configuration>
62+
<deploy.version>1</deploy.version>
63+
</configuration>
6064
</plugin>
6165
</plugins>
6266
</build>
Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<appengine-web-app xmlns="http://appengine.google.com/ns/1.0">
3-
<application>vam-veneer</application>
3+
<application>GCLOUD_PROJECT_ID_HERE</application>
44
<service>flex-compat</service>
55
<version>1</version>
66
<threadsafe>true</threadsafe>
7-
<!--<instance-class>B8</instance-class>-->
87
<env>flex</env>
98
<beta-settings>
109
<setting name="enable_app_engine_apis" value="true"/>
1110
</beta-settings>
1211
<manual-scaling>
1312
<instances>1</instances>
1413
</manual-scaling>
14+
<resources>
15+
<cpu>1</cpu>
16+
<memory-gb>4</memory-gb>
17+
</resources>
1518
</appengine-web-app>

testing/google-cloud-appengine-flex-java/src/main/appengine/app.yaml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
runtime: java
22
env: flex
33

4-
runtime_config: # Optional
5-
jdk: openjdk8
6-
server: jetty9
7-
84
handlers:
95
- url: /.*
106
script: this field is required, but ignored

testing/google-cloud-appengine-java8/src/main/webapp/WEB-INF/appengine-web.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<appengine-web-app xmlns="http://appengine.google.com/ns/1.0">
3-
<application>vam-veneer</application>
3+
<application>GCLOUD_PROJECT_ID_HERE</application>
44
<service>std-java8</service>
55
<version>1</version>
66
<runtime>java8</runtime>

testing/google-cloud-managed-test/src/main/java/com/google/cloud/managed/test/GcjFlexTestServlet.java

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -33,32 +33,33 @@ public class GcjFlexTestServlet extends HttpServlet {
3333

3434
private static final long serialVersionUID = 523885428311420041L;
3535

36-
private final ThreadPoolExecutor executor = new ThreadPoolExecutor(1, 1, 0L,
37-
TimeUnit.MILLISECONDS, new LinkedBlockingQueue<Runnable>());
38-
private volatile GcjTestRunner testRunner = null;
36+
private final ThreadPoolExecutor executor =
37+
new ThreadPoolExecutor(1, 1, 0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<Runnable>());
38+
private volatile GcjTestRunner testRunner;
3939

4040
@Override
4141
protected void doPost(HttpServletRequest req, HttpServletResponse resp)
4242
throws ServletException, IOException {
4343
try {
4444
String classNames = req.getParameter("classes");
4545
PrintWriter out = resp.getWriter();
46-
out.append(startTest(loadClasses(classNames.split("[\\r\\n]+")), req, resp));
46+
List<Class<?>> testClasses = loadClasses(classNames.split("[\\r\\n]+"));
47+
String output = runTests(testClasses, req, resp);
48+
out.append(output);
4749
out.close();
4850
} catch (Exception e) {
4951
throw new RuntimeException(e);
5052
}
5153
}
5254

5355
@Override
54-
public void doGet(HttpServletRequest req, HttpServletResponse resp)
55-
throws IOException {
56+
public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
5657
PrintWriter out = resp.getWriter();
5758
out.append(getTestOutput(req, resp));
5859
out.close();
5960
}
6061

61-
private List<Class<?>> loadClasses(String[] classNames) throws ClassNotFoundException {
62+
private List<Class<?>> loadClasses(String... classNames) throws ClassNotFoundException {
6263
List<Class<?>> classes = new ArrayList<>();
6364
for (String className : classNames) {
6465
String cn = className.trim();
@@ -69,34 +70,44 @@ private List<Class<?>> loadClasses(String[] classNames) throws ClassNotFoundExce
6970
return classes;
7071
}
7172

72-
private String startTest(List<Class<?>> classes, HttpServletRequest req,
73-
HttpServletResponse resp) {
73+
private String runTests(
74+
List<Class<?>> classes, HttpServletRequest req, HttpServletResponse resp) {
7475
synchronized (executor) {
7576
resp.setContentType("text/html");
7677
if (executor.getActiveCount() > 0) {
7778
return "Cannot start new test: the previous test hasn't completed yet.<p/>"
78-
+ "The active test progress is at <a href='" + req.getRequestURL()
79-
+ "'>" + req.getRequestURL() + "</a>.";
79+
+ "The active test progress is at <a href='"
80+
+ req.getRequestURL()
81+
+ "'>"
82+
+ req.getRequestURL()
83+
+ "</a>.";
8084
}
8185

8286
testRunner = new GcjTestRunner(classes);
8387
executor.execute(testRunner);
8488
return "Test started. Check progress at <a href='"
85-
+ req.getRequestURL() + "'>" + req.getRequestURL() + "</a>";
89+
+ req.getRequestURL()
90+
+ "'>"
91+
+ req.getRequestURL()
92+
+ "</a>";
8693
}
8794
}
8895

8996
private String getTestOutput(HttpServletRequest req, HttpServletResponse resp) {
9097
synchronized (executor) {
91-
if (testRunner == null) {
92-
resp.setContentType("text/html");
93-
String link = req.getRequestURL()
94-
.substring(0, req.getRequestURL().length() - req.getRequestURI().length());
95-
return "Test hasn't been started yet. Go to <a href='" + link + "'>" + link
96-
+ "</a> to start a new test";
98+
if (testRunner != null) {
99+
resp.setContentType("text");
100+
return testRunner.getOutput();
97101
}
98-
resp.setContentType("text");
99-
return testRunner.getOutput();
102+
resp.setContentType("text/html");
103+
int urlUriLenDiff = req.getRequestURL().length() - req.getRequestURI().length();
104+
String link = req.getRequestURL().substring(0, urlUriLenDiff);
105+
return "Test hasn't been started yet. Go to <a href='"
106+
+ link
107+
+ "'>"
108+
+ link
109+
+ "</a> to start a new test";
110+
100111
}
101112
}
102113

testing/google-cloud-managed-test/src/main/java/com/google/cloud/managed/test/GcjTestRunner.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,10 @@
1515
*/
1616
package com.google.cloud.managed.test;
1717

18-
import com.google.cloud.storage.BlobIdTest;
1918
import java.io.ByteArrayOutputStream;
2019
import java.io.PrintStream;
2120
import java.io.UnsupportedEncodingException;
2221
import java.util.ArrayList;
23-
import java.util.Collections;
2422
import java.util.List;
2523
import org.junit.internal.TextListener;
2624
import org.junit.runner.JUnitCore;
@@ -44,7 +42,7 @@ public void run() {
4442
synchronized (resultStream) {
4543
resultBytes.reset();
4644
}
47-
for(Class<?> clazz : classes) {
45+
for (Class<?> clazz : classes) {
4846
resultStream.append("\n").append("Running ").append(clazz.getName()).append("\n\n");
4947
unit.run(clazz);
5048
}

utilities/update_pom_version.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ CURRENT_VERSION=$(mvn org.apache.maven.plugins:maven-help-plugin:2.1.1:evaluate
1515
CURRENT_VERSION_BASE=$(mvn org.apache.maven.plugins:maven-help-plugin:2.1.1:evaluate -Dexpression=project.version | grep -Ev '(^\[|\w+:)' | grep -Eo '[0-9]+\.[0-9]+\.[0-9]+')
1616

1717
# Get list of directories for which pom.xml must be updated
18-
module_folders=($(find . -maxdepth 2 -type d | sed -E -n "/^\.\/(google-cloud-contrib\/)?google-cloud(-[a-z]+)+$/p") . ./google-cloud)
18+
module_folders=($(find . -maxdepth 2 -type d | sed -E -n "/^\.\/(google-cloud-contrib\/|testing\/)?google-cloud(-[a-z0-9]+)+$/p") . ./google-cloud)
1919

2020
CURRENT_SNAPSHOT=""
2121
if [ "${CURRENT_VERSION##*-}" == "SNAPSHOT" ]; then

0 commit comments

Comments
 (0)