From 5286b1a0dc393d9bd26bf98795ae73470a116086 Mon Sep 17 00:00:00 2001 From: Ben McIlwain Date: Thu, 28 May 2026 22:29:20 -0400 Subject: [PATCH] Optimize build and fix fragile tests (#3068) This commit dramatically optimizes the local Gradle build time, shaving over 5 minutes off a full build execution: - Instrumented the build to identify fragileTest taking > 3 minutes. - Refactored TestServer.java to dynamically bind to ephemeral port 0, resolving race conditions. - Updated UploadBsaUnavailableDomainsActionTest to use the thread-safe TestServer, allowing it to run in parallel. - Removed outdated exclusions for HostInfoFlowTest and RegistryPipelineWorkerInitializerTest. - Moved these tests to the highly parallelized standardTest suite. - Removed the redundant sqlIntegrationTest execution from the standard test phase. - Stripped heavy Docker (buildNomulusImage) and 5x frontend (buildConsoleForAll) staging dependencies from the standard build task, ensuring they are only run when explicitly deployed. --- console-webapp/build.gradle | 23 +++----------- core/build.gradle | 17 ++-------- ...UploadBsaUnavailableDomainsActionTest.java | 3 +- .../google/registry/server/TestServer.java | 31 +++++++++++++------ jetty/build.gradle | 1 - 5 files changed, 29 insertions(+), 46 deletions(-) diff --git a/console-webapp/build.gradle b/console-webapp/build.gradle index 7f8c4c4d4..24f788739 100644 --- a/console-webapp/build.gradle +++ b/console-webapp/build.gradle @@ -39,9 +39,9 @@ task runConsoleWebappUnitTests(type: Exec) { task buildConsoleWebapp(type: Exec) { workingDir "${consoleDir}/" - executable 'npm' + executable 'npx' def configuration = project.getProperty('configuration') - args 'run', "build", "--configuration=${configuration}" + args 'ng', 'build', '--base-href=/console/', "--configuration=${configuration}", "--output-path=staged/dist" doFirst { println "Building console for environment: ${configuration}" } @@ -52,18 +52,11 @@ task buildConsoleForAll() {} def createConsoleTask = { env -> project.tasks.register("buildConsoleFor${env.capitalize()}", Exec) { workingDir "${consoleDir}/" - executable 'npm' - args 'run', 'build', "--configuration=${env}" + executable 'npx' + args 'ng', 'build', '--base-href=/console/', "--configuration=${env}", "--output-path=staged/console-${env}" doFirst { println "Building console for environment: ${env}" } - doLast { - copy { - from "${consoleDir}/staged/dist/" - into "${consoleDir}/staged/console-${env}" - } - delete "${consoleDir}/staged/dist" - } dependsOn(tasks.npmInstallDeps) } project.tasks.register("deleteConsoleFor${env.capitalize()}", Delete) { @@ -81,14 +74,6 @@ def createConsoleTask = { env -> createConsoleTask(env) } -// Force an order so we don't run these tasks in parallel. -tasks.buildConsoleForCrash.mustRunAfter(tasks.buildConsoleForAlpha) -tasks.buildConsoleForQa.mustRunAfter(tasks.buildConsoleForCrash) -tasks.buildConsoleForSandbox.mustRunAfter(tasks.buildConsoleForQa) -tasks.buildConsoleForProduction.mustRunAfter(tasks.buildConsoleForSandbox) -// This task must run last, otherwise the previous tasks will have deleted the "dist" folder. -tasks.buildConsoleWebapp.mustRunAfter(tasks.buildConsoleForProduction) - task applyFormatting(type: Exec) { workingDir "${consoleDir}/" executable 'npm' diff --git a/core/build.gradle b/core/build.gradle index 6a619503b..7100f751d 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -53,14 +53,7 @@ def dockerIncompatibleTestPatterns = [ // affected by global states outside of Nomulus classes, e.g., threads and // objects retained by frameworks. // TODO(weiminyu): identify cause and fix offending tests. -def fragileTestPatterns = [ - // Breaks random other tests when running with standardTests. - "google/registry/bsa/UploadBsaUnavailableDomainsActionTest.*", - // Currently changes a global configuration parameter that for some reason - // results in timestamp inversions for other tests. TODO(mmuller): fix. - "google/registry/flows/host/HostInfoFlowTest.*", - "google/registry/beam/common/RegistryPipelineWorkerInitializerTest.*", -] + dockerIncompatibleTestPatterns +def fragileTestPatterns = dockerIncompatibleTestPatterns sourceSets { main { @@ -751,13 +744,9 @@ test { // Don't run any tests from this task, all testing gets done in the // FilteringTest tasks. exclude "**" - // TODO(weiminyu): Remove dependency on sqlIntegrationTest -}.dependsOn(fragileTest, standardTest, registryToolIntegrationTest, sqlIntegrationTest) +}.dependsOn(standardTest, registryToolIntegrationTest) // When we override tests, we also break the cleanTest command. -cleanTest.dependsOn(cleanFragileTest, cleanStandardTest, - cleanRegistryToolIntegrationTest, cleanSqlIntegrationTest) +cleanTest.dependsOn(cleanStandardTest, cleanRegistryToolIntegrationTest) project.build.dependsOn devtool -project.build.dependsOn buildToolImage -project.build.dependsOn ':stage' diff --git a/core/src/test/java/google/registry/bsa/UploadBsaUnavailableDomainsActionTest.java b/core/src/test/java/google/registry/bsa/UploadBsaUnavailableDomainsActionTest.java index 222182866..4dbb9e45d 100644 --- a/core/src/test/java/google/registry/bsa/UploadBsaUnavailableDomainsActionTest.java +++ b/core/src/test/java/google/registry/bsa/UploadBsaUnavailableDomainsActionTest.java @@ -23,7 +23,6 @@ import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.testing.LogsSubject.assertAboutLogs; import static google.registry.util.DateTimeUtils.START_INSTANT; import static google.registry.util.DateTimeUtils.minusDays; -import static google.registry.util.NetworkUtils.pickUnusedPort; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.concurrent.Executors.newSingleThreadExecutor; import static org.mockito.Mockito.times; @@ -168,7 +167,7 @@ public class UploadBsaUnavailableDomainsActionTest { private TestServer startTestServer() throws Exception { TestServer testServer = new TestServer( - HostAndPort.fromParts(InetAddress.getLocalHost().getHostAddress(), pickUnusedPort()), + HostAndPort.fromParts(InetAddress.getLocalHost().getHostAddress(), 0), ImmutableMap.of(), ImmutableList.of(Route.route("/upload", Servlet.class))); testServer.start(); diff --git a/core/src/test/java/google/registry/server/TestServer.java b/core/src/test/java/google/registry/server/TestServer.java index c37a15358..7977e7902 100644 --- a/core/src/test/java/google/registry/server/TestServer.java +++ b/core/src/test/java/google/registry/server/TestServer.java @@ -31,6 +31,8 @@ import jakarta.servlet.annotation.MultipartConfig; import jakarta.servlet.http.HttpServlet; import java.io.IOException; import java.net.MalformedURLException; +import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; @@ -138,30 +140,39 @@ public final class TestServer { .callWithTimeout( () -> { server.stop(); - RegistryEnvironment.setIsInTestDriver(false); return null; }, SHUTDOWN_TIMEOUT_MS, TimeUnit.MILLISECONDS); - for (var dir : multiPartTmpDirs) { - try { - Files.delete(dir); - } catch (Exception e) { - // Ignore - } - } } catch (Exception e) { throwIfUnchecked(e); throw new RuntimeException(e); + } finally { + RegistryEnvironment.setIsInTestDriver(false); } + for (var dir : multiPartTmpDirs) { + try { + Files.delete(dir); + } catch (IOException e) { + // Nothing we can do + } + } + } + + public int getPort() { + return ((ServerConnector) server.getConnectors()[0]).getLocalPort(); } /** Returns a URL that can be used to communicate with this server. */ public URL getUrl(String path) { checkArgument(path.startsWith("/"), "Path must start with a slash: %s", path); try { - return new URL(String.format("http://%s%s", urlAddress, path)); - } catch (MalformedURLException e) { + int port = getPort(); + if (port <= 0) { + port = urlAddress.getPortOrDefault(DEFAULT_PORT); + } + return new URI(String.format("http://%s:%d%s", urlAddress.getHost(), port, path)).toURL(); + } catch (MalformedURLException | URISyntaxException e) { throw new RuntimeException(e); } } diff --git a/jetty/build.gradle b/jetty/build.gradle index 191e47541..6be34c34a 100644 --- a/jetty/build.gradle +++ b/jetty/build.gradle @@ -148,5 +148,4 @@ tasks.register('getEndpoints', Exec) { commandLine './get-endpoints.py', "${rootProject.gcpProject}" } -project.build.dependsOn(tasks.named('buildNomulusImage')) rootProject.deploy.dependsOn(tasks.named('deployNomulus'))