Skip to content

Commit

Permalink
Fix tests that assume DAO.findAll result order (#2568)
Browse files Browse the repository at this point in the history
The findAll method on the jOOQ DAO classes doesn't return results in any
specific order. Usually, it happens to return results in insertion order, but,
especially when multiple tests are hitting the database concurrently, the order
can be different.

Fix all the tests that compare findAll's return value to a list of multiple
rows without sorting the list; they now use assertTableEquals instead.
  • Loading branch information
sgrimm authored Nov 5, 2024
1 parent 06317ff commit 317cd43
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 125 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import com.terraformation.backend.db.DatabaseTest
import com.terraformation.backend.db.accelerator.DeliverableType
import com.terraformation.backend.db.accelerator.SubmissionStatus
import com.terraformation.backend.db.accelerator.tables.pojos.SubmissionSnapshotsRow
import com.terraformation.backend.db.accelerator.tables.pojos.SubmissionsRow
import com.terraformation.backend.db.accelerator.tables.records.ParticipantProjectSpeciesRecord
import com.terraformation.backend.db.accelerator.tables.records.SubmissionsRecord
import com.terraformation.backend.db.accelerator.tables.references.SUBMISSIONS
Expand Down Expand Up @@ -199,25 +198,24 @@ class ParticipantProjectSpeciesServiceTest : DatabaseTest(), RunsAsUser {
val userId = currentUser().userId
val now = clock.instant

assertEquals(
assertTableEquals(
listOf(
SubmissionsRow(
SubmissionsRecord(
createdBy = userId,
createdTime = now,
deliverableId = deliverableId,
modifiedBy = userId,
modifiedTime = now,
projectId = projectId1,
submissionStatusId = SubmissionStatus.NotSubmitted),
SubmissionsRow(
SubmissionsRecord(
createdBy = userId,
createdTime = now,
deliverableId = deliverableId,
modifiedBy = userId,
modifiedTime = now,
projectId = projectId2,
submissionStatusId = SubmissionStatus.NotSubmitted)),
submissionsDao.fetchByDeliverableId(deliverableId).map { it.copy(id = null) })
submissionStatusId = SubmissionStatus.NotSubmitted)))

eventPublisher.assertEventsPublished(
existingModels.toSet().map {
Expand Down Expand Up @@ -276,25 +274,24 @@ class ParticipantProjectSpeciesServiceTest : DatabaseTest(), RunsAsUser {
val userId = currentUser().userId
val now = clock.instant

assertEquals(
assertTableEquals(
listOf(
SubmissionsRow(
SubmissionsRecord(
createdBy = userId,
createdTime = now,
deliverableId = deliverableIdMostRecent,
modifiedBy = userId,
modifiedTime = now,
projectId = projectId1,
submissionStatusId = SubmissionStatus.NotSubmitted),
SubmissionsRow(
SubmissionsRecord(
createdBy = userId,
createdTime = now,
deliverableId = deliverableIdMostRecent,
modifiedBy = userId,
modifiedTime = now,
projectId = projectId2,
submissionStatusId = SubmissionStatus.NotSubmitted)),
submissionsDao.fetchByDeliverableId(deliverableIdMostRecent).map { it.copy(id = null) })
submissionStatusId = SubmissionStatus.NotSubmitted)))

eventPublisher.assertEventsPublished(
existingModels.toSet().map {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import com.terraformation.backend.db.accelerator.tables.pojos.ApplicationHistori
import com.terraformation.backend.db.accelerator.tables.pojos.ApplicationModulesRow
import com.terraformation.backend.db.accelerator.tables.pojos.ApplicationsRow
import com.terraformation.backend.db.accelerator.tables.records.ApplicationHistoriesRecord
import com.terraformation.backend.db.accelerator.tables.records.ApplicationModulesRecord
import com.terraformation.backend.db.accelerator.tables.records.ApplicationsRecord
import com.terraformation.backend.db.default_schema.LandUseModelType
import com.terraformation.backend.db.default_schema.OrganizationId
import com.terraformation.backend.db.default_schema.ProjectId
Expand Down Expand Up @@ -103,41 +105,35 @@ class ApplicationStoreTest : DatabaseTest(), RunsAsUser {

val model = store.create(inserted.projectId)

assertEquals(
listOf(
ApplicationsRow(
applicationStatusId = ApplicationStatus.NotSubmitted,
createdBy = user.userId,
createdTime = now,
id = model.id,
internalName = "XXX_Organization 1",
modifiedBy = user.userId,
modifiedTime = now,
projectId = inserted.projectId,
)),
applicationsDao.findAll())
assertTableEquals(
ApplicationsRecord(
applicationStatusId = ApplicationStatus.NotSubmitted,
createdBy = user.userId,
createdTime = now,
id = model.id,
internalName = "XXX_Organization 1",
modifiedBy = user.userId,
modifiedTime = now,
projectId = inserted.projectId))

assertEquals(
listOf(
ApplicationHistoriesRow(
applicationId = model.id,
applicationStatusId = ApplicationStatus.NotSubmitted,
modifiedBy = user.userId,
modifiedTime = now,
)),
applicationHistoriesDao.findAll().map { it.copy(id = null) })
assertTableEquals(
ApplicationHistoriesRecord(
applicationId = model.id,
applicationStatusId = ApplicationStatus.NotSubmitted,
modifiedBy = user.userId,
modifiedTime = now,
))

assertEquals(
assertTableEquals(
listOf(
ApplicationModulesRow(
ApplicationModulesRecord(
applicationId = model.id,
moduleId = prescreenModuleId,
applicationModuleStatusId = ApplicationModuleStatus.Incomplete),
ApplicationModulesRow(
ApplicationModulesRecord(
applicationId = model.id,
moduleId = applicationModuleId,
applicationModuleStatusId = ApplicationModuleStatus.Incomplete)),
applicationModulesDao.findAll())
applicationModuleStatusId = ApplicationModuleStatus.Incomplete)))
}

@Test
Expand Down Expand Up @@ -1445,18 +1441,16 @@ class ApplicationStoreTest : DatabaseTest(), RunsAsUser {
applicationStatusId = ApplicationStatus.PassedPreScreen)),
applicationHistoriesDao.findAll().map { it.copy(id = null) })

assertEquals(
setOf(
ApplicationModulesRow(
assertTableEquals(
listOf(
ApplicationModulesRecord(
applicationId = applicationId,
moduleId = moduleId1,
applicationModuleStatusId = ApplicationModuleStatus.Incomplete),
ApplicationModulesRow(
ApplicationModulesRecord(
applicationId = applicationId,
moduleId = moduleId2,
applicationModuleStatusId = ApplicationModuleStatus.Incomplete),
),
applicationModulesDao.findAll().toSet())
applicationModuleStatusId = ApplicationModuleStatus.Incomplete)))
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.terraformation.backend.accelerator.db
import com.terraformation.backend.RunsAsUser
import com.terraformation.backend.db.DatabaseTest
import com.terraformation.backend.db.accelerator.tables.pojos.DefaultVotersRow
import com.terraformation.backend.db.accelerator.tables.records.DefaultVotersRecord
import com.terraformation.backend.db.default_schema.UserId
import com.terraformation.backend.mockUser
import io.mockk.every
Expand Down Expand Up @@ -90,8 +91,7 @@ class DefaultVoterStoreTest : DatabaseTest(), RunsAsUser {
insertDefaultVoter(user1)
store.insert(user2)

assertEquals(
listOf(DefaultVotersRow(user1), DefaultVotersRow(user2)), defaultVotersDao.findAll())
assertTableEquals(listOf(DefaultVotersRecord(user1), DefaultVotersRecord(user2)))
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import com.terraformation.backend.db.accelerator.DeliverableType
import com.terraformation.backend.db.accelerator.ParticipantProjectSpeciesId
import com.terraformation.backend.db.accelerator.SubmissionStatus
import com.terraformation.backend.db.accelerator.tables.pojos.ParticipantProjectSpeciesRow
import com.terraformation.backend.db.accelerator.tables.records.ParticipantProjectSpeciesRecord
import com.terraformation.backend.db.default_schema.SpeciesNativeCategory
import com.terraformation.backend.mockUser
import com.terraformation.backend.species.model.ExistingSpeciesModel
Expand Down Expand Up @@ -597,9 +598,9 @@ class ParticipantProjectSpeciesStoreTest : DatabaseTest(), RunsAsUser {
val userId = user.userId
val now = Instant.EPOCH

assertEquals(
assertTableEquals(
listOf(
ParticipantProjectSpeciesRow(
ParticipantProjectSpeciesRecord(
createdBy = userId,
createdTime = now,
feedback = null,
Expand All @@ -609,7 +610,7 @@ class ParticipantProjectSpeciesStoreTest : DatabaseTest(), RunsAsUser {
rationale = null,
speciesId = speciesId1,
submissionStatusId = SubmissionStatus.NotSubmitted),
ParticipantProjectSpeciesRow(
ParticipantProjectSpeciesRecord(
createdBy = userId,
createdTime = now,
feedback = null,
Expand All @@ -619,7 +620,7 @@ class ParticipantProjectSpeciesStoreTest : DatabaseTest(), RunsAsUser {
rationale = null,
speciesId = speciesId2,
submissionStatusId = SubmissionStatus.NotSubmitted),
ParticipantProjectSpeciesRow(
ParticipantProjectSpeciesRecord(
createdBy = userId,
createdTime = now,
feedback = null,
Expand All @@ -629,7 +630,7 @@ class ParticipantProjectSpeciesStoreTest : DatabaseTest(), RunsAsUser {
rationale = null,
speciesId = speciesId1,
submissionStatusId = SubmissionStatus.NotSubmitted),
ParticipantProjectSpeciesRow(
ParticipantProjectSpeciesRecord(
createdBy = userId,
createdTime = now,
feedback = null,
Expand All @@ -638,8 +639,7 @@ class ParticipantProjectSpeciesStoreTest : DatabaseTest(), RunsAsUser {
projectId = projectId2,
rationale = null,
speciesId = speciesId2,
submissionStatusId = SubmissionStatus.NotSubmitted)),
participantProjectSpeciesDao.findAll().map { it.copy(id = null) })
submissionStatusId = SubmissionStatus.NotSubmitted)))
}
}

Expand Down Expand Up @@ -796,9 +796,9 @@ class ParticipantProjectSpeciesStoreTest : DatabaseTest(), RunsAsUser {

// If the current user does not have permission to delete any entity in the list,
// the entire delete fails and there are no changes
assertEquals(
assertTableEquals(
listOf(
ParticipantProjectSpeciesRow(
ParticipantProjectSpeciesRecord(
createdBy = userId,
createdTime = now,
feedback = null,
Expand All @@ -811,7 +811,7 @@ class ParticipantProjectSpeciesStoreTest : DatabaseTest(), RunsAsUser {
speciesId = speciesId1,
speciesNativeCategoryId = null,
submissionStatusId = SubmissionStatus.NotSubmitted),
ParticipantProjectSpeciesRow(
ParticipantProjectSpeciesRecord(
createdBy = userId,
createdTime = now,
feedback = null,
Expand All @@ -823,8 +823,7 @@ class ParticipantProjectSpeciesStoreTest : DatabaseTest(), RunsAsUser {
rationale = null,
speciesId = speciesId2,
speciesNativeCategoryId = null,
submissionStatusId = SubmissionStatus.NotSubmitted)),
participantProjectSpeciesDao.findAll())
submissionStatusId = SubmissionStatus.NotSubmitted)))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ import com.terraformation.backend.db.default_schema.OrganizationType
import com.terraformation.backend.db.default_schema.Role
import com.terraformation.backend.db.default_schema.UserId
import com.terraformation.backend.db.default_schema.UserType
import com.terraformation.backend.db.default_schema.tables.pojos.OrganizationManagedLocationTypesRow
import com.terraformation.backend.db.default_schema.tables.pojos.OrganizationsRow
import com.terraformation.backend.db.default_schema.tables.pojos.UserPreferencesRow
import com.terraformation.backend.db.default_schema.tables.records.OrganizationManagedLocationTypesRecord
import com.terraformation.backend.db.default_schema.tables.references.USER_PREFERENCES
import com.terraformation.backend.mockUser
import io.mockk.every
Expand Down Expand Up @@ -285,18 +285,14 @@ internal class OrganizationStoreTest : DatabaseTest(), RunsAsUser {
val createdModel =
store.createWithAdmin(row, setOf(ManagedLocationType.Nursery, ManagedLocationType.SeedBank))

val expected =
assertTableEquals(
listOf(
OrganizationManagedLocationTypesRow(
OrganizationManagedLocationTypesRecord(
organizationId = createdModel.id,
managedLocationTypeId = ManagedLocationType.Nursery),
OrganizationManagedLocationTypesRow(
OrganizationManagedLocationTypesRecord(
organizationId = createdModel.id,
managedLocationTypeId = ManagedLocationType.SeedBank))

val actual = organizationManagedLocationTypesDao.findAll()

assertEquals(expected, actual)
managedLocationTypeId = ManagedLocationType.SeedBank)))
}

@Test
Expand Down Expand Up @@ -814,18 +810,14 @@ internal class OrganizationStoreTest : DatabaseTest(), RunsAsUser {
val createdModel =
store.createWithAdmin(row, setOf(ManagedLocationType.Nursery, ManagedLocationType.SeedBank))

val expected =
assertTableEquals(
listOf(
OrganizationManagedLocationTypesRow(
OrganizationManagedLocationTypesRecord(
organizationId = createdModel.id,
managedLocationTypeId = ManagedLocationType.Nursery),
OrganizationManagedLocationTypesRow(
OrganizationManagedLocationTypesRecord(
organizationId = createdModel.id,
managedLocationTypeId = ManagedLocationType.SeedBank))

val actual = organizationManagedLocationTypesDao.findAll()

assertEquals(expected, actual)
managedLocationTypeId = ManagedLocationType.SeedBank)))

store.delete(createdModel.id)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import com.terraformation.backend.db.default_schema.Role
import com.terraformation.backend.db.default_schema.UserId
import com.terraformation.backend.db.default_schema.UserType
import com.terraformation.backend.db.default_schema.tables.pojos.UserGlobalRolesRow
import com.terraformation.backend.db.default_schema.tables.records.UserGlobalRolesRecord
import com.terraformation.backend.db.default_schema.tables.records.UserPreferencesRecord
import com.terraformation.backend.db.default_schema.tables.references.USER_PREFERENCES
import com.terraformation.backend.dummyKeycloakInfo
Expand Down Expand Up @@ -865,11 +866,10 @@ internal class UserStoreTest : DatabaseTest(), RunsAsUser {

userStore.updateGlobalRoles(setOf(userId1, userId2), setOf(GlobalRole.SuperAdmin))

assertEquals(
assertTableEquals(
listOf(
UserGlobalRolesRow(userId = userId1, globalRoleId = GlobalRole.SuperAdmin),
UserGlobalRolesRow(userId = userId2, globalRoleId = GlobalRole.SuperAdmin)),
userGlobalRolesDao.findAll())
UserGlobalRolesRecord(userId = userId1, globalRoleId = GlobalRole.SuperAdmin),
UserGlobalRolesRecord(userId = userId2, globalRoleId = GlobalRole.SuperAdmin)))
}

@Test
Expand Down
Loading

0 comments on commit 317cd43

Please sign in to comment.