-
Notifications
You must be signed in to change notification settings - Fork 5
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
HAI Remove @MockkBean and @SpykBean annotations
Configure the Spring Context in integration tests to use a mockk instance of CableReportService so the mock can be `@Autowired`. CableReportService was defined in several integration tests as a `@MockkBean` dependency. The actual implementation was only used in `CableReportServiceITests`, which constructs its own custom instance. The other class that was injected with `@MockkBean` was `EmailSenderService`. Since there's an easy-to-use extension for mocking an actual email server, it felt better to use that instead of mocking the service universally. Also, unlike `CableReportServiceITests`, `EmailSenderServiceITest` uses the Spring context, so a context with the actual implementation is needed anyway. Each unique combination of `@MockkBeans` and `@SpykBeans` (or the Mockito equilevants) makes Spring create a new context for the test. While not a huge time sink, creating (and destroying) the contexts adds up. We can clearly see how many different contexts are created from the number of SQL connection pools that need to be shut down at the end of the tests. There are situations when they have to be used, but using alternative approaches is usually better. New contexts are also created whenever the configuration parameters are different between tests. This is sometimes unavoidable when we need to test how the application behaves with different configurations. Ever since #397 we've been able to use application.test.properties to configure the test environment, so we don't need to use test-specific properties to diverge from the production configuration. There was a properties parameter like remaining in `TestDataServiceITest`, so that was removed. There's one more service test with custom properties (`EmailSenderServicePropertiesITest`), but in that test we're interested in testing that the properties have the effect we want. And the properties are different from how we generally want to configure EmailSenderService in tests. So the custom properties are unavoidable. Controller tests are cheaper to create with varying configurations. Since only a very limited context is created for running them, recreating it is not very expensive. So testing things like feature flags on the controller level doesn't seem problematic.
- Loading branch information
Showing
16 changed files
with
134 additions
and
144 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ package fi.hel.haitaton.hanke.application | |
| import assertk.Assert | ||
| import assertk.all | ||
| import assertk.assertThat | ||
| import assertk.assertions.contains | ||
| import assertk.assertions.containsExactly | ||
| import assertk.assertions.containsExactlyInAnyOrder | ||
| import assertk.assertions.each | ||
|
|
@@ -21,8 +22,6 @@ import assertk.assertions.prop | |
| import com.icegreen.greenmail.configuration.GreenMailConfiguration | ||
| import com.icegreen.greenmail.junit5.GreenMailExtension | ||
| import com.icegreen.greenmail.util.ServerSetupTest | ||
| import com.ninjasquad.springmockk.MockkBean | ||
| import com.ninjasquad.springmockk.SpykBean | ||
| import fi.hel.haitaton.hanke.DatabaseTest | ||
| import fi.hel.haitaton.hanke.HankeEntity | ||
| import fi.hel.haitaton.hanke.HankeNotFoundException | ||
|
|
@@ -43,8 +42,8 @@ import fi.hel.haitaton.hanke.application.ApplicationType.CABLE_REPORT | |
| import fi.hel.haitaton.hanke.asJsonResource | ||
| import fi.hel.haitaton.hanke.asUtc | ||
| import fi.hel.haitaton.hanke.domain.Hankealue | ||
| import fi.hel.haitaton.hanke.email.ApplicationNotificationData | ||
| import fi.hel.haitaton.hanke.email.EmailSenderService | ||
| import fi.hel.haitaton.hanke.email.EmailSenderService.Companion.translations | ||
| import fi.hel.haitaton.hanke.email.textBody | ||
| import fi.hel.haitaton.hanke.factory.AlluDataFactory | ||
| import fi.hel.haitaton.hanke.factory.AlluDataFactory.Companion.asianHoitajaCustomerContact | ||
| import fi.hel.haitaton.hanke.factory.AlluDataFactory.Companion.cableReportWithoutHanke | ||
|
|
@@ -82,6 +81,7 @@ import fi.hel.haitaton.hanke.permissions.Kayttooikeustaso | |
| import fi.hel.haitaton.hanke.permissions.Kayttooikeustaso.HAKEMUSASIOINTI | ||
| import fi.hel.haitaton.hanke.permissions.PermissionService | ||
| import fi.hel.haitaton.hanke.permissions.kayttajaTunnistePattern | ||
| import fi.hel.haitaton.hanke.test.Asserts.hasReceivers | ||
| import fi.hel.haitaton.hanke.test.Asserts.hasSingleGeometryWithCoordinates | ||
| import fi.hel.haitaton.hanke.test.Asserts.isRecent | ||
| import fi.hel.haitaton.hanke.test.AuditLogEntryEntityAsserts.hasUserActor | ||
|
|
@@ -98,6 +98,7 @@ import io.mockk.justRun | |
| import io.mockk.verify | ||
| import io.mockk.verifyOrder | ||
| import io.mockk.verifySequence | ||
| import jakarta.mail.internet.MimeMessage | ||
| import java.time.OffsetDateTime | ||
| import java.time.ZonedDateTime | ||
| import org.geojson.Polygon | ||
|
|
@@ -132,12 +133,10 @@ private val dataWithoutAreas = createCableReportApplicationData(areas = listOf() | |
| @WithMockUser(USERNAME) | ||
| class ApplicationServiceITest : DatabaseTest() { | ||
|
|
||
| @MockkBean private lateinit var cableReportServiceAllu: CableReportService | ||
| @SpykBean private lateinit var emailSenderService: EmailSenderService | ||
|
|
||
| @Autowired private lateinit var applicationService: ApplicationService | ||
| @Autowired private lateinit var hankeService: HankeService | ||
| @Autowired private lateinit var permissionService: PermissionService | ||
| @Autowired private lateinit var cableReportServiceAllu: CableReportService | ||
|
|
||
| @Autowired private lateinit var applicationRepository: ApplicationRepository | ||
| @Autowired private lateinit var hankeRepository: HankeRepository | ||
|
|
@@ -638,15 +637,10 @@ class ApplicationServiceITest : DatabaseTest() { | |
| USERNAME, | ||
| ) | ||
| setAlluFields(applicationRepository.findById(application.id!!).orElseThrow()) | ||
| val capturedNotifications = mutableListOf<ApplicationNotificationData>() | ||
| justRun { cableReportServiceAllu.update(21, any()) } | ||
| every { cableReportServiceAllu.getApplicationInformation(any()) } returns | ||
| createAlluApplicationResponse(21) | ||
| justRun { cableReportServiceAllu.addAttachment(21, any()) } | ||
| justRun { emailSenderService.sendHankeInvitationEmail(any()) } | ||
| justRun { | ||
| emailSenderService.sendApplicationNotificationEmail(capture(capturedNotifications)) | ||
| } | ||
| val updatedApplication = | ||
| cableReport.copy( | ||
| representativeWithContacts = asianHoitajaCustomerContact, | ||
|
|
@@ -657,11 +651,11 @@ class ApplicationServiceITest : DatabaseTest() { | |
|
|
||
| applicationService.updateApplicationData(application.id!!, updatedApplication, USERNAME) | ||
|
|
||
| val capturedNotifications = getApplicationNotifications() | ||
| assertThat(capturedNotifications) | ||
| .areValid(application.applicationType, application.hankeTunnus) | ||
| assertThat(capturedNotifications) | ||
| .extracting { it.recipientEmail } | ||
| .containsExactlyInAnyOrder( | ||
| assertThat(capturedNotifications.toTypedArray()) | ||
| .hasReceivers( | ||
| "[email protected]", | ||
| asianHoitajaCustomerContact.contacts[0].email, | ||
| rakennuttajaCustomerContact.contacts[0].email | ||
|
|
@@ -671,8 +665,6 @@ class ApplicationServiceITest : DatabaseTest() { | |
| cableReportServiceAllu.update(21, any()) | ||
| cableReportServiceAllu.addAttachment(any(), any()) | ||
| } | ||
| verify(exactly = 3) { emailSenderService.sendHankeInvitationEmail(any()) } | ||
| verify(exactly = 3) { emailSenderService.sendApplicationNotificationEmail(any()) } | ||
| } | ||
|
|
||
| @Test | ||
|
|
@@ -714,7 +706,7 @@ class ApplicationServiceITest : DatabaseTest() { | |
| cableReportServiceAllu.update(21, any()) | ||
| cableReportServiceAllu.addAttachment(any(), any()) | ||
| } | ||
| verify { emailSenderService wasNot Called } | ||
| assertThat(greenMail.receivedMessages).isEmpty() | ||
| } | ||
|
|
||
| @Test | ||
|
|
@@ -1198,16 +1190,14 @@ class ApplicationServiceITest : DatabaseTest() { | |
| CableReportWithoutHanke(CABLE_REPORT, cableReportData), | ||
| USERNAME, | ||
| ) | ||
| val capturedEmails = mutableListOf<ApplicationNotificationData>() | ||
| every { cableReportServiceAllu.create(any()) } returns 26 | ||
| every { cableReportServiceAllu.getApplicationInformation(any()) } returns | ||
| createAlluApplicationResponse(26) | ||
| justRun { cableReportServiceAllu.addAttachment(26, any()) } | ||
| justRun { emailSenderService.sendHankeInvitationEmail(any()) } | ||
| justRun { emailSenderService.sendApplicationNotificationEmail(capture(capturedEmails)) } | ||
|
|
||
| applicationService.sendApplication(application.id!!, USERNAME) | ||
|
|
||
| val capturedEmails = getApplicationNotifications() | ||
| assertThat(capturedEmails).hasSize(3) // 4 contacts, but one is the sender | ||
| assertThat(capturedEmails) | ||
| .areValid(application.applicationType, application.hankeTunnus) | ||
|
|
@@ -1216,8 +1206,6 @@ class ApplicationServiceITest : DatabaseTest() { | |
| cableReportServiceAllu.addAttachment(any(), any()) | ||
| cableReportServiceAllu.getApplicationInformation(any()) | ||
| } | ||
| verify(exactly = 3) { emailSenderService.sendHankeInvitationEmail(any()) } | ||
| verify(exactly = 3) { emailSenderService.sendApplicationNotificationEmail(any()) } | ||
| } | ||
|
|
||
| @Test | ||
|
|
@@ -1856,16 +1844,24 @@ class ApplicationServiceITest : DatabaseTest() { | |
| } | ||
| } | ||
|
|
||
| private fun Assert<List<ApplicationNotificationData>>.areValid( | ||
| type: ApplicationType, | ||
| hankeTunnus: String? | ||
| ) = each { data -> | ||
| data.transform { it.senderEmail }.isEqualTo(hakijaContact.email) | ||
| data.transform { it.senderName }.isEqualTo(hakijaContact.name) | ||
| data.transform { it.applicationIdentifier }.isEqualTo(defaultApplicationIdentifier) | ||
| data.transform { it.applicationType }.isEqualTo(type) | ||
| data.transform { it.recipientEmail }.isIn(*expectedRecipients) | ||
| data.transform { it.hankeTunnus }.isEqualTo(hankeTunnus) | ||
| private fun getApplicationNotifications() = | ||
| greenMail.receivedMessages.filter { | ||
| it.subject.startsWith("Haitaton: Sinut on lisätty hakemukselle") | ||
| } | ||
|
|
||
| private fun Assert<List<MimeMessage>>.areValid(type: ApplicationType, hankeTunnus: String?) { | ||
| each { it.isValid(type, hankeTunnus) } | ||
| } | ||
|
|
||
| private fun Assert<MimeMessage>.isValid(type: ApplicationType, hankeTunnus: String?) { | ||
| prop(MimeMessage::textBody).all { | ||
| contains("${hakijaContact.name} (${hakijaContact.email}) on tehnyt") | ||
| contains("hakemukselle $defaultApplicationIdentifier") | ||
| contains("on tehnyt ${type.translations().fi}") | ||
| contains("hankkeella $hankeTunnus") | ||
| } | ||
|
|
||
| transform { it.allRecipients.first().toString() }.isIn(*expectedRecipients) | ||
| } | ||
|
|
||
| private fun initializedHanke(): HankeEntity = | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
11 changes: 11 additions & 0 deletions
11
.../src/integrationTest/kotlin/fi/hel/haitaton/hanke/configuration/MockCableReportService.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package fi.hel.haitaton.hanke.configuration | ||
|
|
||
| import fi.hel.haitaton.hanke.allu.CableReportService | ||
| import io.mockk.mockk | ||
| import org.springframework.context.annotation.Bean | ||
| import org.springframework.context.annotation.Configuration | ||
|
|
||
| @Configuration | ||
| class MockCableReportService { | ||
| @Bean fun cableReportService(): CableReportService = mockk() | ||
| } |
Oops, something went wrong.