PDJB-NONE: [EXPERIMENT] Block external analytics requests in integration tests

.github/instructions/database.instructions.md

@@ -101,6 +101,13 @@ CREATE TABLE example_table (
 CREATE INDEX idx_example_name ON example_table(name);
 ```
 
+### Reference Data in Migrations
+
+Integration tests reset the database by truncating every table except `flyway_schema_history` and `local_council`.
+If a migration inserts reference data that tests rely on, add that table to `PRESERVED_TABLES` in
+`src/test/kotlin/uk/gov/communities/prsdb/webapp/testHelpers/IntegrationTestHelper.kt`. Otherwise the data will be
+deleted before every test that resets the database.
+
 ## Search with Trigrams
 - Use `pg_trgm` extension for fuzzy text search
 - See existing search implementations for patterns

.github/workflows/build-and-test.yml

@@ -51,7 +51,17 @@ jobs:
         env:
           EMAILNOTIFICATIONS_APIKEY: ${{ secrets.NOTIFY_KEY }}
           EMAILNOTIFICATIONS_USE_PRODUCTION_NOTIFY: ${{ github.base_ref == 'production' || github.merge_group.base_ref == 'refs/head/production' }}
-        run: ./gradlew clean check --no-daemon
+        run: ./gradlew clean check --no-daemon -PtestForks=2
+
+      # TEMPORARY, experiment branch only: gives each CI run per-class timings rather than a single
+      # noisy wall-clock number, so changes under ~10% can be judged against CI's ~2 min variance.
+      - name: Upload test results
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: test-results-${{ github.run_attempt }}
+          path: build/test-results/test/*.xml
+          retention-days: 5
 
       - name: Run ktlintFormat
         run: ./gradlew ktlintFormat --no-daemon

build.gradle.kts

@@ -1,5 +1,6 @@
 import org.gradle.internal.os.OperatingSystem
 import org.jetbrains.kotlin.gradle.tasks.KotlinCompile
+import java.util.concurrent.ConcurrentLinkedQueue
 
 plugins {
     kotlin("jvm") version "1.9.25"
@@ -159,10 +160,47 @@ tasks.withType<KotlinCompile> {
     dependsOn("copyBuiltAssets")
 }
 
+val testForks = (project.findProperty("testForks") as String?)?.toInt() ?: 1
+val slowTestCount = (project.findProperty("slowTestCount") as String?)?.toInt() ?: 15
+
 tasks.withType<Test> {
     useJUnitPlatform()
     dependsOn("copyBuiltAssets")
     maxHeapSize = "2g"
+    maxParallelForks = testForks
+
+    val taskLogger = logger
+    val testDurations = ConcurrentLinkedQueue<Pair<String, Long>>()
+
+    addTestListener(
+        object : TestListener {
+            override fun beforeSuite(suite: TestDescriptor) = Unit
+
+            override fun beforeTest(testDescriptor: TestDescriptor) = Unit
+
+            override fun afterTest(
+                testDescriptor: TestDescriptor,
+                result: TestResult,
+            ) {
+                val name = "${testDescriptor.className}.${testDescriptor.name}"
+                testDurations.add(name to (result.endTime - result.startTime))
+            }
+
+            override fun afterSuite(
+                suite: TestDescriptor,
+                result: TestResult,
+            ) {
+                if (suite.parent != null) return
+                val slowest = testDurations.sortedByDescending { it.second }.take(slowTestCount)
+                if (slowest.isEmpty()) return
+                taskLogger.lifecycle("")
+                taskLogger.lifecycle("Slowest ${slowest.size} tests:")
+                slowest.forEach { (name, durationMs) ->
+                    taskLogger.lifecycle(String.format("  %8.2fs  %s", durationMs / 1000.0, name))
+                }
+            }
+        },
+    )
 }
 
 tasks.register<JavaExec>("playwright") {

docs/IntegrationTestReadMe.md

@@ -13,6 +13,27 @@ We set seed data by passing SQL script names into integration test class constru
 Tests that require different seed data to the rest of the class must be put in nested classes that inherit from 
 `NestedIntegrationTestWithMutableData` or `NestedIntegrationTestWithImmutableData` depending on the outer class.
 
+Resetting the database means truncating every table in the `public` schema with `RESTART IDENTITY CASCADE`, then
+running the seed scripts. Two tables are deliberately preserved:
+
+* `flyway_schema_history` - Flyway's own bookkeeping. Dropping it would make every migration re-run.
+* `local_council` - reference data inserted by `V1_0_0__la_and_address_tables.sql` (as `local_authority`, renamed by
+  `V1_6_0`). No seed script writes to it.
+
+These are the only two tables with rows after a bare migrate. If you add a migration that inserts reference data, add
+its table to `PRESERVED_TABLES` in `IntegrationTestHelper`, or the data will be wiped before every test. You can check
+which tables hold reference data by migrating an empty database and looking for non-empty tables.
+
+## Test Timing
+
+Every run ends with a table of the slowest tests, so that suite speed stays visible as scenarios are added. Change
+its length with `-PslowTestCount=N`.
+
+Running the suite across parallel Gradle forks was measured and rejected. Each fork needs its own Spring contexts,
+Postgres and Redis containers, application instance and browser, and Playwright tests are latency-sensitive, so the
+contention inflates per-test time faster than the parallelism recovers it: summed test time rose 32% at two forks and
+84% at three, making wall clock no better than serial while introducing timeout and Docker-discovery flakes.
+
 ## Page Objects (and Components)
 
 We encapsulate logic for interacting with our pages into page objects (as described by [Martin Fowler](https://martinfowler.com/bliki/PageObject.html) and

src/test/kotlin/uk/gov/communities/prsdb/webapp/TestcontainersConfiguration.kt

@@ -9,11 +9,39 @@ import org.testcontainers.utility.DockerImageName
 
 @TestConfiguration(proxyBeanMethods = false)
 class TestcontainersConfiguration {
+    companion object {
+        // One Postgres and one Redis per JVM, shared by every Spring context. Without this, each of the
+        // ~16 distinct integration contexts starts its own pair and a serial run ends with 32 containers
+        // alive, none of which are ever released.
+        //
+        // stop() is deliberately a no-op. Spring Boot closes container beans when their context is
+        // destroyed, and the test context cache is LRU with a default maxSize of 32 against more contexts
+        // than that in a full run. An eviction would otherwise stop these containers out from under the
+        // contexts still using them. Testcontainers' Ryuk sidecar removes them when the JVM exits.
+        private val postgres: PostgreSQLContainer<*> =
+            object : PostgreSQLContainer<Nothing>(DockerImageName.parse("postgres:latest")) {
+                override fun stop() = Unit
+            }.apply {
+                // One container now serves every Spring context, and each context keeps its own Hikari
+                // pool, so Postgres' default limit of 100 connections is exhausted once ~10 contexts exist.
+                setCommand("postgres", "-c", "max_connections=400")
+                start()
+            }
+
+        private val redis: GenericContainer<*> =
+            object : GenericContainer<Nothing>(DockerImageName.parse("redis:latest")) {
+                override fun stop() = Unit
+            }.apply {
+                addExposedPort(6379)
+                start()
+            }
+    }
+
     @Bean
     @ServiceConnection
-    fun postgresContainer(): PostgreSQLContainer<*> = PostgreSQLContainer(DockerImageName.parse("postgres:latest"))
+    fun postgresContainer(): PostgreSQLContainer<*> = postgres
 
     @Bean
     @ServiceConnection(name = "redis")
-    fun redisContainer(): GenericContainer<*> = GenericContainer(DockerImageName.parse("redis:latest")).withExposedPorts(6379)
+    fun redisContainer(): GenericContainer<*> = redis
 }

src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTest.kt

@@ -34,10 +34,7 @@ import uk.gov.service.notify.NotificationClient
 import kotlin.reflect.full.isSubclassOf
 
 @Import(TestcontainersConfiguration::class)
-@SpringBootTest(
-    webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
-    properties = ["spring.flyway.clean-disabled=false"],
-)
+@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
 @TestInstance(TestInstance.Lifecycle.PER_CLASS)
 @UsePlaywright
 @ActiveProfiles(profiles = ["local", "local-no-auth"])
@@ -135,6 +132,7 @@ abstract class IntegrationTest {
 
     @BeforeEach
     fun setUp(page: Page) {
+        blockExternalAnalytics(page)
         navigator = Navigator(page, port)
     }
 
@@ -161,7 +159,16 @@ abstract class IntegrationTest {
 
     fun createPageAndNavigator(browserContext: BrowserContext): Pair<Page, Navigator> {
         val page = browserContext.newPage()
+        blockExternalAnalytics(page)
         val navigator = Navigator(page, port)
         return Pair(page, navigator)
     }
+
+    // The layout renders <script async src="https://plausible.io/js/pa-...js"> from a hardcoded constant, so
+    // every page load reaches out to the internet. That script delays the window load event, which
+    // ClickAndWaitable.clickAndWait waits for, putting an external round trip on the critical path of every
+    // interaction. Each test gets a fresh BrowserContext, so the browser cache never helps.
+    private fun blockExternalAnalytics(page: Page) {
+        page.route("**plausible.io**") { it.abort() }
+    }
 }

src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTestHelperTests.kt

@@ -0,0 +1,59 @@
+package uk.gov.communities.prsdb.webapp.integration
+
+import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Assertions.assertTrue
+import org.junit.jupiter.api.Test
+import org.springframework.beans.factory.annotation.Autowired
+import org.springframework.jdbc.core.JdbcTemplate
+import uk.gov.communities.prsdb.webapp.testHelpers.IntegrationTestHelper
+
+class IntegrationTestHelperTests : IntegrationTestWithMutableData("data-local.sql") {
+    @Autowired
+    lateinit var jdbcTemplate: JdbcTemplate
+
+    private fun countRowsIn(table: String) = jdbcTemplate.queryForObject("SELECT COUNT(*) FROM $table", Int::class.java)!!
+
+    @Test
+    fun `resetDatabase clears seeded data`() {
+        assertTrue(countRowsIn("prsdb_user") > 0, "seed data should have been loaded")
+
+        IntegrationTestHelper.resetDatabase(jdbcTemplate)
+
+        assertEquals(0, countRowsIn("prsdb_user"))
+        assertEquals(0, countRowsIn("landlord"))
+        assertEquals(0, countRowsIn("property_ownership"))
+    }
+
+    @Test
+    fun `resetDatabase preserves local council reference data`() {
+        val localCouncilsBefore = countRowsIn("local_council")
+        assertTrue(localCouncilsBefore > 0, "migrations should have inserted local councils")
+
+        IntegrationTestHelper.resetDatabase(jdbcTemplate)
+
+        assertEquals(localCouncilsBefore, countRowsIn("local_council"))
+    }
+
+    @Test
+    fun `resetDatabase preserves the flyway schema history`() {
+        val migrationsBefore = countRowsIn("flyway_schema_history")
+        assertTrue(migrationsBefore > 0, "migrations should have been recorded")
+
+        IntegrationTestHelper.resetDatabase(jdbcTemplate)
+
+        assertEquals(migrationsBefore, countRowsIn("flyway_schema_history"))
+    }
+
+    @Test
+    fun `resetDatabase restarts identity sequences`() {
+        IntegrationTestHelper.resetDatabase(jdbcTemplate)
+
+        val id =
+            jdbcTemplate.queryForObject(
+                "INSERT INTO address (single_line_address, postcode) VALUES ('1 Test Street, EG1 2AB', 'EG1 2AB') RETURNING id",
+                Long::class.java,
+            )
+
+        assertEquals(1L, id)
+    }
+}

src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTestWithImmutableData.kt

@@ -1,6 +1,5 @@
 package uk.gov.communities.prsdb.webapp.integration
 
-import org.flywaydb.core.Flyway
 import org.junit.jupiter.api.BeforeAll
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.jdbc.core.JdbcTemplate
@@ -13,10 +12,9 @@ abstract class IntegrationTestWithImmutableData(
 
     @BeforeAll
     fun setUpBeforeAll(
-        @Autowired flyway: Flyway,
         @Autowired jdbcTemplate: JdbcTemplate,
     ) {
-        IntegrationTestHelper.resetAndSeedDatabase(flyway, seedDataScripts, jdbcTemplate)
+        IntegrationTestHelper.resetAndSeedDatabase(seedDataScripts, jdbcTemplate)
     }
 
     abstract class NestedIntegrationTestWithImmutableData(
@@ -26,10 +24,9 @@ abstract class IntegrationTestWithImmutableData(
 
         @BeforeAll
         fun setUpBeforeAll(
-            @Autowired flyway: Flyway,
             @Autowired jdbcTemplate: JdbcTemplate,
         ) {
-            IntegrationTestHelper.resetAndSeedDatabase(flyway, seedDataScripts, jdbcTemplate)
+            IntegrationTestHelper.resetAndSeedDatabase(seedDataScripts, jdbcTemplate)
         }
     }
 }

src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTestWithMutableData.kt

@@ -1,6 +1,5 @@
 package uk.gov.communities.prsdb.webapp.integration
 
-import org.flywaydb.core.Flyway
 import org.junit.jupiter.api.BeforeEach
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.jdbc.core.JdbcTemplate
@@ -13,10 +12,9 @@ abstract class IntegrationTestWithMutableData(
 
     @BeforeEach
     fun setUpBeforeEach(
-        @Autowired flyway: Flyway,
         @Autowired jdbcTemplate: JdbcTemplate,
     ) {
-        IntegrationTestHelper.resetAndSeedDatabase(flyway, seedDataScripts, jdbcTemplate)
+        IntegrationTestHelper.resetAndSeedDatabase(seedDataScripts, jdbcTemplate)
     }
 
     abstract class NestedIntegrationTestWithMutableData(
@@ -26,10 +24,9 @@ abstract class IntegrationTestWithMutableData(
 
         @BeforeEach
         fun setUpBeforeEach(
-            @Autowired flyway: Flyway,
             @Autowired jdbcTemplate: JdbcTemplate,
         ) {
-            IntegrationTestHelper.resetAndSeedDatabase(flyway, seedDataScripts, jdbcTemplate)
+            IntegrationTestHelper.resetAndSeedDatabase(seedDataScripts, jdbcTemplate)
         }
     }
 }

src/test/kotlin/uk/gov/communities/prsdb/webapp/testHelpers/IntegrationTestHelper.kt

@@ -1,23 +1,37 @@
 package uk.gov.communities.prsdb.webapp.testHelpers
 
-import org.flywaydb.core.Flyway
 import org.springframework.jdbc.core.JdbcTemplate
 import org.springframework.util.ResourceUtils
 
 class IntegrationTestHelper {
     companion object {
+        // Tables that must survive a reset. flyway_schema_history is Flyway's own bookkeeping - dropping it
+        // would cause every migration to re-run on the next context start. local_council holds reference
+        // data inserted by V1_0_0__la_and_address_tables.sql (as local_authority, renamed by V1_6_0) and is
+        // never written to by a seed script. These are the only two tables with rows after a bare migrate.
+        // Any future migration that inserts reference data must add its table to this set.
+        private val PRESERVED_TABLES = setOf("flyway_schema_history", "local_council")
+
         fun resetAndSeedDatabase(
-            flyway: Flyway,
             scripts: List<String>,
             jdbcTemplate: JdbcTemplate,
         ) {
-            resetDatabase(flyway)
+            resetDatabase(jdbcTemplate)
             seedDatabase(scripts, jdbcTemplate)
         }
 
-        private fun resetDatabase(flyway: Flyway) {
-            flyway.clean()
-            flyway.migrate()
+        fun resetDatabase(jdbcTemplate: JdbcTemplate) {
+            val tablesToTruncate =
+                jdbcTemplate
+                    .queryForList("SELECT tablename FROM pg_tables WHERE schemaname = 'public'", String::class.java)
+                    .filterNot { it in PRESERVED_TABLES }
+
+            check(tablesToTruncate.isNotEmpty()) {
+                "No truncatable tables found in the public schema - have the Flyway migrations run?"
+            }
+
+            val quotedTables = tablesToTruncate.joinToString(", ") { "\"$it\"" }
+            jdbcTemplate.execute("TRUNCATE TABLE $quotedTables RESTART IDENTITY CASCADE")
         }
 
         private fun seedDatabase(