Build manifestmergetool for integration testing. Tune timeouts for remote builder tests

Author: ekharkunov

server/README.md

@@ -114,6 +114,13 @@ Test can be run from the root directory with
 ```sh
 ./gradlew server:test
 ```
+Unless `-PexcludeTags=integration` is passed, this first builds `server:bootJar` and
+`manifestmergetool:mainJar` into `server/app/` - the directory the test containers bind-mount.
+Both are needed: the containers run `extender.jar`, and `manifestmergetool.jar` is invoked as a
+subprocess by the sdk's `manifestMergeCmd` (see `IntegrationTest.buildAndroidLocalAar`). Those two
+jars are declared outputs of their tasks, so deleting one from `server/app/` re-runs the task
+rather than being silently reported as `UP-TO-DATE`.
+
 During the testing local servers will be run. That's why it necessary to have prebuild docker images. There are two set of services that run:
 * **test** - run for integration testing (see *IntegrationTest.java*)
 * **auth-test** - run for authentication testing (see *AuthenticationTest.java*)

server/build.gradle

@@ -154,6 +154,9 @@ compileJava {
 bootJar {
     archiveBaseName = 'extender'
     version = project.ext.extenderVersion
+    // See the note on :manifestmergetool:mainJar - server/app/extender.jar is what the docker
+    // containers run, so it is declared as an output and restored if deleted.
+    outputs.file("${project.projectDir}/app/extender.jar")
     doLast {
         copy {
             from "${project.projectDir}/build/libs/extender-${project.ext.extenderVersion}.jar"
@@ -201,12 +204,14 @@ task buildStandalone() {
             }
             into "build/standalone"
         }
+        // .archiveFile, not the task itself: a task reference resolves to *all* of its outputs,
+        // which now includes the server/app/ copy declared above.
         copy {
-            from bootJar
+            from bootJar.archiveFile
             into "build/standalone"
         }
         copy {
-            from project(':manifestmergetool').collect { it.tasks.withType(Jar) }
+            from project(':manifestmergetool').tasks.withType(Jar).collect { it.archiveFile }
             into "build/standalone"
         }
     }
@@ -218,6 +223,14 @@ test {
             excludeTags project.getProperty("excludeTags")
         }
     }
+    // The integration and auth tests boot the docker stack, which bind-mounts server/app/ into
+    // every container: extender.jar is what they run, and manifestmergetool.jar is invoked as a
+    // subprocess by the sdk's manifestMergeCmd (e.g. IntegrationTest.buildAndroidLocalAar).
+    // CI's coverage job passes -PexcludeTags=integration and skips the jar build entirely.
+    def excludedTags = (project.findProperty("excludeTags") ?: "").toString().split(",")*.trim()
+    if (!excludedTags.contains("integration")) {
+        dependsOn 'bootJar', ':manifestmergetool:mainJar'
+    }
     // Opt-in knobs to shrink a local integration test run. All absent in CI, where they are no-ops.
     //   -PtargetPlatforms=x86_64-linux,js-web   only build these targets, and only boot the
     //                                           builders they need (see docker-compose.yml profiles)

server/manifestmergetool/build.gradle

@@ -45,6 +45,11 @@ task mainJar(type: Jar) {
     }
     duplicatesStrategy = DuplicatesStrategy.WARN
     with jar
+    // The copy below is a real output of this task: server/app/ is what the docker containers
+    // bind-mount, and the sdk's manifestMergeCmd runs this jar from there. Declaring it means a
+    // deleted server/app/manifestmergetool.jar re-runs the task instead of reporting UP-TO-DATE
+    // and leaving it missing.
+    outputs.file("${project.projectDir}/../app/manifestmergetool.jar")
         doLast {
         copy {
             from "${project.projectDir}/build/libs/manifestmergetool-${project.ext.manifestMergetoolVersion}.jar"

server/scripts/start-test-server.sh

@@ -38,6 +38,16 @@ echo "Using PORT: ${PORT}"
 
 URL=http://localhost:${PORT}
 
+# server/app/ is bind-mounted into every container: extender.jar is what they run, and
+# manifestmergetool.jar is invoked as a subprocess by the sdk's manifestMergeCmd. Missing jars
+# would only surface much later as an obscure in-container failure.
+for jar in extender.jar manifestmergetool.jar; do
+	if [ ! -f "${DIR}/../app/${jar}" ]; then
+		echo "ERROR: server/app/${jar} is missing. Run: ./gradlew server:bootJar manifestmergetool:mainJar"
+		exit 1
+	fi
+done
+
 COMPOSE_FILES=(-f "${DIR}/../docker/docker-compose.yml")
 if [ "$EXTENDER_DEV_CACHE" == "1" ]; then
 	# The volume is external so that the 'extender-test' and 'extender-test-auth'

server/src/test/java/com/defold/extender/remote/RemoteEngineBuilderTest.java

@@ -21,11 +21,14 @@
 import static org.mockito.Mockito.verify;
 
 import java.io.File;
+import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.ArrayList;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
@@ -40,6 +43,8 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.Timeout;
 import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.api.parallel.Execution;
+import org.junit.jupiter.api.parallel.ExecutionMode;
 
 import org.apache.http.HttpEntity;
 
@@ -58,11 +63,21 @@
 import io.micrometer.tracing.propagation.Propagator;
 import io.micrometer.tracing.test.simple.SimpleTracer;
 
+// The methods below start a WireMock server each and assert on wall-clock deadlines. Running
+// them concurrently (junit-platform.properties sets parallel.mode.default=concurrent) puts as
+// many Jetty startups as there are cores on top of each other, which on a loaded CI runner
+// delays the first response past the socket timeout and fails the test with a spurious
+// "Failed to communicate with Extender service".
+@Execution(ExecutionMode.SAME_THREAD)
 public class RemoteEngineBuilderTest {
 
     private static final long BUILD_SLEEP_TIMEOUT = 100;
     private static final long BUILD_RESULT_WAIT_TIMEOUT = 10_000;
-    private static final int SOCKET_TIMEOUT = 2_000;
+    // Generous enough that a busy CI runner is never mistaken for a network fault: a
+    // SocketTimeoutException is not retried by the client and marks the build as failed.
+    private static final int SOCKET_TIMEOUT = 30_000;
+    // Only for the test that has to give up on a deliberately stalled response.
+    private static final int SHORT_SOCKET_TIMEOUT = 2_000;
 
     @TempDir
     Path tmpDir;
@@ -71,6 +86,7 @@ public class RemoteEngineBuilderTest {
     private ExecutorService executor;
     private Path resultLocation;
     private SimpleMeterRegistry meterRegistry;
+    private final List<RemoteEngineBuilder> builders = new ArrayList<>();
 
     @BeforeEach
     public void setUp() throws Exception {
@@ -88,6 +104,16 @@ public void setUp() throws Exception {
     @AfterEach
     public void tearDown() {
         executor.shutdownNow();
+        // Each client owns connection eviction threads, which would otherwise leak for the
+        // remainder of the suite. Closing is cleanup only, so it never fails a test.
+        for (RemoteEngineBuilder builder : builders) {
+            try {
+                builder.httpClient.close();
+            } catch (IOException e) {
+                // ignore
+            }
+        }
+        builders.clear();
         builderMock.stop();
     }
 
@@ -101,20 +127,28 @@ private RemoteEngineBuilder createBuilder(int resultDownloadRetries, long buildR
 
     private RemoteEngineBuilder createBuilder(Optional<GCPInstanceService> instanceService,
                                               int resultDownloadRetries, long buildResultWaitTimeout) {
-        return new RemoteEngineBuilder(
+        return createBuilder(instanceService, resultDownloadRetries, buildResultWaitTimeout, SOCKET_TIMEOUT);
+    }
+
+    private RemoteEngineBuilder createBuilder(Optional<GCPInstanceService> instanceService,
+                                              int resultDownloadRetries, long buildResultWaitTimeout,
+                                              int socketTimeout) {
+        RemoteEngineBuilder builder = new RemoteEngineBuilder(
             instanceService,
             resultLocation.toString(),
             BUILD_SLEEP_TIMEOUT,
             buildResultWaitTimeout,
-            SOCKET_TIMEOUT,
-            SOCKET_TIMEOUT,
-            SOCKET_TIMEOUT,
+            socketTimeout,
+            socketTimeout,
+            socketTimeout,
             35,
             resultDownloadRetries,
             100,
             meterRegistry,
             new SimpleTracer(),
             Propagator.NOOP);
+        builders.add(builder);
+        return builder;
     }
 
     private double reconnectCount(String operation) {
@@ -170,7 +204,10 @@ public void stalledResultDownloadMustNotBlockOtherBuilds() throws Exception {
         builderMock.stubFor(get(urlPathEqualTo("/job_result"))
                 .willReturn(aResponse().withStatus(200).withFixedDelay(30_000).withBody(new byte[] {0x42})));
 
-        RemoteEngineBuilder builder = createBuilder(1);
+        // The short socket timeout is what lets the stuck downloads give up on the 30s delayed
+        // response inside this test's deadline.
+        RemoteEngineBuilder builder = createBuilder(Optional.empty(), 1, BUILD_RESULT_WAIT_TIMEOUT,
+            SHORT_SOCKET_TIMEOUT);
 
         // Occupy the connection pool with builds stuck downloading their result
         Path stalledResult1 = submitBuild(builder, "stalled1");