From a86e686f0be9290f5ba30c8cf9398f27868342ab Mon Sep 17 00:00:00 2001 From: Steven Grimm <1248649+sgrimm@users.noreply.github.com> Date: Mon, 4 Nov 2024 10:17:47 -0800 Subject: [PATCH] Improve assertTableEquals usability (#2567) Make a few improvements to the assertTableEquals functions: * The "include primary keys" flag is now inferred from the list of expected records; callers no longer have to specify it explicitly. * The expected records can now be any kind of collection, not just a Set. * The lists of expected and actual records are now sorted before they're passed to assertEquals, rather than passed as sets. This should make it easier to spot the differences in assertion failure messages, especially when the test is run in an IDE that can diff the expected and actual values. --- .../ParticipantProjectSpeciesServiceTest.kt | 6 +-- .../backend/db/DatabaseBackedTest.kt | 46 ++++++++----------- .../backend/nursery/db/BatchImporterTest.kt | 4 +- 3 files changed, 22 insertions(+), 34 deletions(-) diff --git a/src/test/kotlin/com/terraformation/backend/accelerator/ParticipantProjectSpeciesServiceTest.kt b/src/test/kotlin/com/terraformation/backend/accelerator/ParticipantProjectSpeciesServiceTest.kt index 3d6594f5503b..faa19b1130ec 100644 --- a/src/test/kotlin/com/terraformation/backend/accelerator/ParticipantProjectSpeciesServiceTest.kt +++ b/src/test/kotlin/com/terraformation/backend/accelerator/ParticipantProjectSpeciesServiceTest.kt @@ -121,8 +121,7 @@ class ParticipantProjectSpeciesServiceTest : DatabaseTest(), RunsAsUser { modifiedBy = userId, modifiedTime = now, projectId = projectId, - submissionStatusId = SubmissionStatus.NotSubmitted), - includePrimaryKeys = false) + submissionStatusId = SubmissionStatus.NotSubmitted)) assertTableEquals( ParticipantProjectSpeciesRecord( @@ -134,8 +133,7 @@ class ParticipantProjectSpeciesServiceTest : DatabaseTest(), RunsAsUser { projectId = projectId, rationale = "rationale", speciesId = speciesId, - submissionStatusId = SubmissionStatus.NotSubmitted), - includePrimaryKeys = false) + submissionStatusId = SubmissionStatus.NotSubmitted)) eventPublisher.assertEventPublished( ParticipantProjectSpeciesAddedEvent( diff --git a/src/test/kotlin/com/terraformation/backend/db/DatabaseBackedTest.kt b/src/test/kotlin/com/terraformation/backend/db/DatabaseBackedTest.kt index d585223e02e3..07bd99a289d4 100644 --- a/src/test/kotlin/com/terraformation/backend/db/DatabaseBackedTest.kt +++ b/src/test/kotlin/com/terraformation/backend/db/DatabaseBackedTest.kt @@ -3102,25 +3102,29 @@ abstract class DatabaseBackedTest { * You will usually want one of the other variants of this that require fewer parameters. * * @param table The table whose contents should be examined. - * @param expected The set of records the table should contain. + * @param expected The records the table should contain. If the records' primary key fields are + * null, the primary keys of the table records will be ignored. * @param message Assertion failure message; defaults to the table name. * @param where Optional query condition to assert on a subset of a table's contents. Only rows * matching the condition will be considered. - * @param includePrimaryKeys If false, the primary key field(s) of the records from the database - * will be cleared before comparing against [expected]. This can be used to ignore - * database-generated IDs. * @param transform Function to apply to records from the database before comparing them to * [expected]. Can be used to clear or hardwire specific fields that aren't relevant to the * behavior being tested. */ protected fun > assertTableEquals( table: Table, - expected: Set, + expected: Collection, message: String = table.name, where: Condition? = null, - includePrimaryKeys: Boolean = true, transform: ((R) -> R)? = null, ) { + val sampleExpected = expected.firstOrNull() + + // If the expected records have primary key values, then include primary keys in the records + // that get pulled from the database. This is not the Kotlin data class copy(), but the one from + // UpdatableRecord, which clears primary key fields. + val includePrimaryKeys = sampleExpected != sampleExpected?.copy() + val actual = dslContext .selectFrom(table) @@ -3131,36 +3135,32 @@ abstract class DatabaseBackedTest { if (includePrimaryKeys) { transformed } else { - // This is not the Kotlin data class copy(), but the one from UpdatableRecord, which - // clears primary key fields. + // This is UpdatableRecord.copy(). which clears primary key fields. transformed.copy() } } - .toSet() + .sorted() - assertEquals(expected, actual, message) + assertEquals(expected.sorted(), actual, message) } /** * Asserts that a table (or a filtered subset of it) contains an expected set of records and * nothing else. * - * @param expected The set of records the table should contain. Must be non-empty. + * @param expected The records the table should contain. Must be non-empty. If the records' + * primary key fields are null, the primary keys of the table records will be ignored. * @param message Assertion failure message; defaults to the table name. * @param where Optional query condition to assert on a subset of a table's contents. Only rows * matching the condition will be considered. - * @param includePrimaryKeys If false, the primary key field(s) of the records from the database - * will be cleared before comparing against [expected]. This can be used to ignore - * database-generated IDs. * @param transform Function to apply to records from the database before comparing them to * [expected]. Can be used to clear or hardwire specific fields that aren't relevant to the * behavior being tested. */ protected fun > assertTableEquals( - expected: Set, + expected: Collection, message: String? = null, where: Condition? = null, - includePrimaryKeys: Boolean = true, transform: ((R) -> R)? = null, ) { val table = @@ -3172,7 +3172,6 @@ abstract class DatabaseBackedTest { expected = expected, message = message ?: table.name, where = where, - includePrimaryKeys = includePrimaryKeys, transform = transform, ) } @@ -3180,13 +3179,11 @@ abstract class DatabaseBackedTest { /** * Asserts that a table (or a filtered subset of it) contains exactly one row. * - * @param expected The single row that the table should contain. + * @param expected The single record that the table should contain. If its primary key field(s) + * are null, the primary keys of the table records will be ignored. * @param message Assertion failure message; defaults to the table name. * @param where Optional query condition to assert on a subset of a table's contents. Only rows * matching the condition will be considered. - * @param includePrimaryKeys If false, the primary key field(s) of the records from the database - * will be cleared before comparing against [expected]. This can be used to ignore - * database-generated IDs. * @param transform Function to apply to records from the database before comparing them to * [expected]. Can be used to clear or hardwire specific fields that aren't relevant to the * behavior being tested. @@ -3195,15 +3192,10 @@ abstract class DatabaseBackedTest { expected: R, message: String? = null, where: Condition? = null, - includePrimaryKeys: Boolean = true, transform: ((R) -> R)? = null, ) { assertTableEquals( - expected = setOf(expected), - message = message, - where = where, - includePrimaryKeys = includePrimaryKeys, - transform = transform) + expected = listOf(expected), message = message, where = where, transform = transform) } /** diff --git a/src/test/kotlin/com/terraformation/backend/nursery/db/BatchImporterTest.kt b/src/test/kotlin/com/terraformation/backend/nursery/db/BatchImporterTest.kt index cfb23a1f3e90..cde0528cdc91 100644 --- a/src/test/kotlin/com/terraformation/backend/nursery/db/BatchImporterTest.kt +++ b/src/test/kotlin/com/terraformation/backend/nursery/db/BatchImporterTest.kt @@ -301,9 +301,7 @@ internal class BatchImporterTest : DatabaseTest(), RunsAsUser { position = 2, typeId = UploadProblemType.MalformedValue, uploadId = uploadId, - value = "ShortName", - ), - includePrimaryKeys = false) + value = "ShortName")) assertStatus(UploadStatus.Invalid) }