From 2be3d32b00b3d5c48befbe71b1085d087491d85a Mon Sep 17 00:00:00 2001 From: Travis Woodward <104013113+Travis-Softwire@users.noreply.github.com> Date: Thu, 30 Jul 2026 13:52:10 +0100 Subject: [PATCH 1/8] PDJB-NONE: Add truncate-based database reset for integration tests --- .../integration/IntegrationTestHelperTests.kt | 59 +++++++++++++++++++ .../testHelpers/IntegrationTestHelper.kt | 21 +++++++ 2 files changed, 80 insertions(+) create mode 100644 src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTestHelperTests.kt diff --git a/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTestHelperTests.kt b/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTestHelperTests.kt new file mode 100644 index 0000000000..d57d96f9c8 --- /dev/null +++ b/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) + } +} diff --git a/src/test/kotlin/uk/gov/communities/prsdb/webapp/testHelpers/IntegrationTestHelper.kt b/src/test/kotlin/uk/gov/communities/prsdb/webapp/testHelpers/IntegrationTestHelper.kt index edbb7ae989..5357143f3b 100644 --- a/src/test/kotlin/uk/gov/communities/prsdb/webapp/testHelpers/IntegrationTestHelper.kt +++ b/src/test/kotlin/uk/gov/communities/prsdb/webapp/testHelpers/IntegrationTestHelper.kt @@ -6,6 +6,13 @@ 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, @@ -15,6 +22,20 @@ class IntegrationTestHelper { seedDatabase(scripts, jdbcTemplate) } + 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 resetDatabase(flyway: Flyway) { flyway.clean() flyway.migrate() From 0c4200239f66e563fb543c195f033704453a0165 Mon Sep 17 00:00:00 2001 From: Travis Woodward <104013113+Travis-Softwire@users.noreply.github.com> Date: Thu, 30 Jul 2026 13:59:59 +0100 Subject: [PATCH 2/8] PDJB-NONE: Reset integration test database by truncation instead of Flyway clean --- .../prsdb/webapp/integration/IntegrationTest.kt | 5 +---- .../integration/IntegrationTestWithImmutableData.kt | 7 ++----- .../webapp/integration/IntegrationTestWithMutableData.kt | 7 ++----- .../prsdb/webapp/testHelpers/IntegrationTestHelper.kt | 9 +-------- 4 files changed, 6 insertions(+), 22 deletions(-) diff --git a/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTest.kt b/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTest.kt index c255e5bc72..e9a7129028 100644 --- a/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTest.kt +++ b/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"]) diff --git a/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTestWithImmutableData.kt b/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTestWithImmutableData.kt index 7beec3e42a..fccdff7ad1 100644 --- a/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTestWithImmutableData.kt +++ b/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) } } } diff --git a/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTestWithMutableData.kt b/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTestWithMutableData.kt index 74a9a939e9..7f5adcbffc 100644 --- a/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTestWithMutableData.kt +++ b/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) } } } diff --git a/src/test/kotlin/uk/gov/communities/prsdb/webapp/testHelpers/IntegrationTestHelper.kt b/src/test/kotlin/uk/gov/communities/prsdb/webapp/testHelpers/IntegrationTestHelper.kt index 5357143f3b..e38aae7c5a 100644 --- a/src/test/kotlin/uk/gov/communities/prsdb/webapp/testHelpers/IntegrationTestHelper.kt +++ b/src/test/kotlin/uk/gov/communities/prsdb/webapp/testHelpers/IntegrationTestHelper.kt @@ -1,6 +1,5 @@ package uk.gov.communities.prsdb.webapp.testHelpers -import org.flywaydb.core.Flyway import org.springframework.jdbc.core.JdbcTemplate import org.springframework.util.ResourceUtils @@ -14,11 +13,10 @@ class IntegrationTestHelper { private val PRESERVED_TABLES = setOf("flyway_schema_history", "local_council") fun resetAndSeedDatabase( - flyway: Flyway, scripts: List, jdbcTemplate: JdbcTemplate, ) { - resetDatabase(flyway) + resetDatabase(jdbcTemplate) seedDatabase(scripts, jdbcTemplate) } @@ -36,11 +34,6 @@ class IntegrationTestHelper { jdbcTemplate.execute("TRUNCATE TABLE $quotedTables RESTART IDENTITY CASCADE") } - private fun resetDatabase(flyway: Flyway) { - flyway.clean() - flyway.migrate() - } - private fun seedDatabase( scripts: List, jdbcTemplate: JdbcTemplate, From 6878a4f4305f4a0d7278f4d2af18c2761b437bc6 Mon Sep 17 00:00:00 2001 From: Travis Woodward <104013113+Travis-Softwire@users.noreply.github.com> Date: Thu, 30 Jul 2026 14:09:01 +0100 Subject: [PATCH 3/8] PDJB-NONE: Run the test suite across three parallel Gradle forks --- build.gradle.kts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/build.gradle.kts b/build.gradle.kts index ddf77022b8..95beed4079 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -159,10 +159,16 @@ tasks.withType { dependsOn("copyBuiltAssets") } +// Three forks is the useful maximum: Gradle schedules by top-level class, every nested grouping in the +// integration tests is a JUnit @Nested inner class, and PropertyRegistrationSinglePageTests alone is a +// single scheduling unit worth ~5 minutes. Use -PtestForks=1 to fall back to serial execution. +val testForks = (project.findProperty("testForks") as String?)?.toInt() ?: 3 + tasks.withType { useJUnitPlatform() dependsOn("copyBuiltAssets") maxHeapSize = "2g" + maxParallelForks = testForks } tasks.register("playwright") { From ea8ccdb31285781b54ea87c4fb7c354d6256e4f2 Mon Sep 17 00:00:00 2001 From: Travis Woodward <104013113+Travis-Softwire@users.noreply.github.com> Date: Thu, 30 Jul 2026 14:20:39 +0100 Subject: [PATCH 4/8] PDJB-NONE: Report the slowest tests after each test run --- build.gradle.kts | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/build.gradle.kts b/build.gradle.kts index 95beed4079..8d5f53a147 100644 --- a/build.gradle.kts +++ b/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" @@ -163,12 +164,46 @@ tasks.withType { // integration tests is a JUnit @Nested inner class, and PropertyRegistrationSinglePageTests alone is a // single scheduling unit worth ~5 minutes. Use -PtestForks=1 to fall back to serial execution. val testForks = (project.findProperty("testForks") as String?)?.toInt() ?: 3 +val slowTestCount = (project.findProperty("slowTestCount") as String?)?.toInt() ?: 15 tasks.withType { useJUnitPlatform() dependsOn("copyBuiltAssets") maxHeapSize = "2g" maxParallelForks = testForks + + val taskLogger = logger + val testDurations = ConcurrentLinkedQueue>() + + 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("playwright") { From e8ee1c2121e90b99ed669b4009600af528f8df1b Mon Sep 17 00:00:00 2001 From: Travis Woodward <104013113+Travis-Softwire@users.noreply.github.com> Date: Thu, 30 Jul 2026 14:22:01 +0100 Subject: [PATCH 5/8] PDJB-NONE: Document the integration test reset and parallel fork behaviour --- .github/instructions/database.instructions.md | 7 ++++++ docs/IntegrationTestReadMe.md | 23 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/.github/instructions/database.instructions.md b/.github/instructions/database.instructions.md index b2a01c516d..dc815003ba 100644 --- a/.github/instructions/database.instructions.md +++ b/.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 diff --git a/docs/IntegrationTestReadMe.md b/docs/IntegrationTestReadMe.md index 11d230593f..4fd67d98cb 100644 --- a/docs/IntegrationTestReadMe.md +++ b/docs/IntegrationTestReadMe.md @@ -13,6 +13,29 @@ 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. + +## Parallel Execution + +Tests run across three forked JVMs. Each fork is completely isolated: its own Spring contexts, its own Postgres and +Redis containers, its own database and its own browser. No test needs to be aware of this. + +Three is the useful maximum because Gradle schedules by top-level class, every nested grouping is a JUnit +`@Nested inner class`, and `PropertyRegistrationSinglePageTests` alone is a single scheduling unit worth about five +minutes. + +Override the fork count with `-PtestForks=N`. Use `-PtestForks=1` for fully serial execution when debugging. Every run +ends with a table of the slowest tests; change its length with `-PslowTestCount=N`. + ## 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 From ac083bc77fd52016bbaaa23b934227720ee59718 Mon Sep 17 00:00:00 2001 From: Travis Woodward <104013113+Travis-Softwire@users.noreply.github.com> Date: Thu, 30 Jul 2026 16:32:46 +0100 Subject: [PATCH 6/8] PDJB-NONE: Remove parallel test forks after measuring them as a net loss --- build.gradle.kts | 5 ----- docs/IntegrationTestReadMe.md | 16 +++++++--------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 8d5f53a147..636b100fa9 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -160,17 +160,12 @@ tasks.withType { dependsOn("copyBuiltAssets") } -// Three forks is the useful maximum: Gradle schedules by top-level class, every nested grouping in the -// integration tests is a JUnit @Nested inner class, and PropertyRegistrationSinglePageTests alone is a -// single scheduling unit worth ~5 minutes. Use -PtestForks=1 to fall back to serial execution. -val testForks = (project.findProperty("testForks") as String?)?.toInt() ?: 3 val slowTestCount = (project.findProperty("slowTestCount") as String?)?.toInt() ?: 15 tasks.withType { useJUnitPlatform() dependsOn("copyBuiltAssets") maxHeapSize = "2g" - maxParallelForks = testForks val taskLogger = logger val testDurations = ConcurrentLinkedQueue>() diff --git a/docs/IntegrationTestReadMe.md b/docs/IntegrationTestReadMe.md index 4fd67d98cb..28f30df005 100644 --- a/docs/IntegrationTestReadMe.md +++ b/docs/IntegrationTestReadMe.md @@ -24,17 +24,15 @@ These are the only two tables with rows after a bare migrate. If you add a migra 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. -## Parallel Execution +## Test Timing -Tests run across three forked JVMs. Each fork is completely isolated: its own Spring contexts, its own Postgres and -Redis containers, its own database and its own browser. No test needs to be aware of this. +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`. -Three is the useful maximum because Gradle schedules by top-level class, every nested grouping is a JUnit -`@Nested inner class`, and `PropertyRegistrationSinglePageTests` alone is a single scheduling unit worth about five -minutes. - -Override the fork count with `-PtestForks=N`. Use `-PtestForks=1` for fully serial execution when debugging. Every run -ends with a table of the slowest tests; 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) From c6bd4dd3be2a3cdac4ec52793243a6f63bc4a429 Mon Sep 17 00:00:00 2001 From: Travis Woodward <104013113+Travis-Softwire@users.noreply.github.com> Date: Fri, 31 Jul 2026 16:21:01 +0100 Subject: [PATCH 7/8] PDJB-NONE: Block external analytics in tests and upload CI test results --- .github/workflows/build-and-test.yml | 10 ++++++++++ .../prsdb/webapp/integration/IntegrationTest.kt | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 9fa6f11af1..87d1461f57 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -53,5 +53,15 @@ jobs: EMAILNOTIFICATIONS_USE_PRODUCTION_NOTIFY: ${{ github.base_ref == 'production' || github.merge_group.base_ref == 'refs/head/production' }} run: ./gradlew clean check --no-daemon + # 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 diff --git a/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTest.kt b/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTest.kt index e9a7129028..01d0976461 100644 --- a/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTest.kt +++ b/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/IntegrationTest.kt @@ -132,6 +132,7 @@ abstract class IntegrationTest { @BeforeEach fun setUp(page: Page) { + blockExternalAnalytics(page) navigator = Navigator(page, port) } @@ -158,7 +159,16 @@ abstract class IntegrationTest { fun createPageAndNavigator(browserContext: BrowserContext): Pair { val page = browserContext.newPage() + blockExternalAnalytics(page) val navigator = Navigator(page, port) return Pair(page, navigator) } + + // The layout renders