feat: refactor work with spring boot test context#3561
Conversation
📝 WalkthroughWalkthroughThis pull request consolidates test infrastructure by centralizing mock bean configurations (particularly PostHog), removes per-test configuration imports, implements test sharding via CI workflows and JUnit 5 extensions, refactors database cleanup logic, and updates property configurations across multiple test classes for consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Gradle as Gradle Build
participant JUnit as JUnit 5
participant TestClass as Test Class
GH->>GH: Create matrix with shard_index & shard_total
GH->>Gradle: Invoke test task with SHARD_INDEX & SHARD_TOTAL env vars
Gradle->>Gradle: Read env vars, set JVM system properties
Gradle->>JUnit: Initialize JUnit Platform with system properties
JUnit->>TestShardCondition: Load extension via service loader
TestShardCondition->>TestShardCondition: Compute shard for test class (hash % total)
TestShardCondition->>JUnit: Return enabled/disabled condition
JUnit->>TestClass: Execute or skip test based on shard assignment
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2c1dd16 to
748f642
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/app/src/test/kotlin/io/tolgee/service/KeyServiceTest.kt (1)
21-25:⚠️ Potential issue | 🟡 MinorTest method name is misleading.
The method is named
deletes language with ai resultsbut actually deletes a key viakeyService.hardDelete(). Consider renaming to accurately describe what's being tested.Also, per coding guidelines, test classes should clean up test data in
@AfterEach. This test saves data viatestDataService.saveTestData()but has no cleanup method.Suggested fixes
+import org.junit.jupiter.api.AfterEach + class KeyServiceTest : AbstractSpringTest() { + `@AfterEach` + fun cleanup() { + testDataService.cleanAllTestData() + } + `@Test` `@Transactional` - fun `deletes language with ai results`() { + fun `deletes key with ai results`() { val testData = PromptTestData() testDataService.saveTestData(testData.root) keyService.hardDelete(testData.keys[0].self.id) } }Based on learnings: "Use TestData classes for test setup following the pattern: create TestData instance, save with testDataService, and clean in AfterEach"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/src/test/kotlin/io/tolgee/service/KeyServiceTest.kt` around lines 21 - 25, Rename the test method `deletes language with ai results` to reflect that it deletes a key (e.g., `deletes key with ai results`) and update any references to the test name; ensure the test uses the intended TestData fixture `PromptTestData()` and actions (`testDataService.saveTestData(...)`, `keyService.hardDelete(...)`) remain correct. Also add an `@AfterEach` cleanup method in the test class that calls the test data teardown path used elsewhere (e.g., invoke the testDataService cleanup for the saved PromptTestData) so saved data is removed after each test; reference the class-level TestData usage and the testDataService to implement the cleanup.backend/app/src/test/kotlin/io/tolgee/service/UserAccountServiceTest.kt (1)
21-25:⚠️ Potential issue | 🟡 MinorTest method name is misleading.
The method is named
deletes language with ai resultsbut actually deletes a user account viauserAccountService.delete(). This appears to be copy-paste from another test. Consider renaming to accurately describe what's being tested.Also, per coding guidelines, test classes should clean up test data in
@AfterEach.Suggested fixes
+import org.junit.jupiter.api.AfterEach + class UserAccountServiceTest : AbstractSpringTest() { + `@AfterEach` + fun cleanup() { + testDataService.cleanAllTestData() + } + `@Test` `@Transactional` - fun `deletes language with ai results`() { + fun `deletes user account with ai results`() { val testData = PromptTestData() testDataService.saveTestData(testData.root) userAccountService.delete(testData.organizationMember.self.id) } }Based on learnings: "Use TestData classes for test setup following the pattern: create TestData instance, save with testDataService, and clean in AfterEach"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/src/test/kotlin/io/tolgee/service/UserAccountServiceTest.kt` around lines 21 - 25, The test method name `deletes language with ai results` is incorrect for the behavior (it calls userAccountService.delete), so rename the test in UserAccountServiceTest to something accurate like `deletes user account` and keep using PromptTestData/TestData pattern (create PromptTestData instance, call testDataService.saveTestData(testData.root) before invoking userAccountService.delete(testData.organizationMember.self.id)). Add an `@AfterEach` teardown in the test class that cleans the created TestData (e.g. fun tearDown() { testDataService.deleteTestData(testData.root) } or the project's equivalent cleanup method) so test data is removed after each test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/testing/src/main/kotlin/io/tolgee/CleanDbTestListener.kt`:
- Around line 26-30: The field nonEmptyCheckQuery lacks `@Volatile` causing
visibility races with cachedTables; mark nonEmptyCheckQuery as `@Volatile` too so
writes in getAllTables are visible to other threads that read cachedTables and
nonEmptyCheckQuery (refer to companion object fields cachedTables and
nonEmptyCheckQuery and the getAllTables method in CleanDbTestListener) to ensure
consistent visibility across threads.
In
`@backend/testing/src/main/kotlin/io/tolgee/config/TestPostHogConfiguration.kt`:
- Around line 11-15: The global PostHog mock bean (provided by
TestPostHogConfiguration.testPostHog) is not reset between tests, causing
invocation leakage; update AbstractSpringTest's `@BeforeEach` setup method (the
method that currently clears caches) to call Mockito.reset(postHog) —
referencing the PostHog bean/field named postHog in AbstractSpringTest — so the
PostHog mock is cleared before each test run.
In `@backend/testing/src/main/kotlin/io/tolgee/testing/TestShardCondition.kt`:
- Line 36: The current computation in TestShardCondition uses
testClass.name.hashCode().absoluteValue % shardTotal which fails for
Int.MIN_VALUE (absoluteValue stays negative) and can yield a negative modulo;
change the hash calculation to mask off the sign bit instead of using
absoluteValue (i.e., take testClass.name.hashCode() and bitwise-and with
Int.MAX_VALUE to guarantee a non-negative int) and then apply % shardTotal so
hash is always non-negative.
In
`@backend/testing/src/main/resources/META-INF/services/org.junit.jupiter.api.extension.Extension`:
- Line 1: The META-INF service registration references a missing JUnit extension
class TestShardCondition; either implement io.tolgee.testing.TestShardCondition
as a JUnit 5 ExecutionCondition (implement
org.junit.jupiter.api.extension.ExecutionCondition and its
evaluateExecutionCondition(ExtensionContext) returning
ConditionEvaluationResult.enabled/disabled based on your shard logic) or remove
the service registration line; if implementing, ensure the class is in package
io.tolgee.testing, is on the test classpath, and contains the decision logic
used by your tests (or simply return enabled for a no-op placeholder).
---
Outside diff comments:
In `@backend/app/src/test/kotlin/io/tolgee/service/KeyServiceTest.kt`:
- Around line 21-25: Rename the test method `deletes language with ai results`
to reflect that it deletes a key (e.g., `deletes key with ai results`) and
update any references to the test name; ensure the test uses the intended
TestData fixture `PromptTestData()` and actions
(`testDataService.saveTestData(...)`, `keyService.hardDelete(...)`) remain
correct. Also add an `@AfterEach` cleanup method in the test class that calls the
test data teardown path used elsewhere (e.g., invoke the testDataService cleanup
for the saved PromptTestData) so saved data is removed after each test;
reference the class-level TestData usage and the testDataService to implement
the cleanup.
In `@backend/app/src/test/kotlin/io/tolgee/service/UserAccountServiceTest.kt`:
- Around line 21-25: The test method name `deletes language with ai results` is
incorrect for the behavior (it calls userAccountService.delete), so rename the
test in UserAccountServiceTest to something accurate like `deletes user account`
and keep using PromptTestData/TestData pattern (create PromptTestData instance,
call testDataService.saveTestData(testData.root) before invoking
userAccountService.delete(testData.organizationMember.self.id)). Add an
`@AfterEach` teardown in the test class that cleans the created TestData (e.g. fun
tearDown() { testDataService.deleteTestData(testData.root) } or the project's
equivalent cleanup method) so test data is removed after each test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8d3d07bf-a8a4-42e1-a431-8b63a33ce825
📒 Files selected for processing (28)
.github/workflows/test.ymlbackend/app/build.gradlebackend/app/src/test/kotlin/io/tolgee/activity/ActivityLogTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/BusinessEventControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/UserMfaControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2InvitationControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2UserControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/notification/NotificationSettingsControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/organizationController/OrganizationControllerInvitingTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ExportController/V2ExportControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerInvitationTest.ktbackend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.ktbackend/app/src/test/kotlin/io/tolgee/controllers/MarketingEmailingTest.ktbackend/app/src/test/kotlin/io/tolgee/controllers/PublicControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/controllers/resetPassword/ResetPasswordControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/mcp/McpServerIntegrationTest.ktbackend/app/src/test/kotlin/io/tolgee/security/EmailVerificationTest.ktbackend/app/src/test/kotlin/io/tolgee/service/KeyServiceTest.ktbackend/app/src/test/kotlin/io/tolgee/service/LanguageServiceTest.ktbackend/app/src/test/kotlin/io/tolgee/service/UserAccountServiceTest.ktbackend/app/src/test/kotlin/io/tolgee/service/notification/NotificationServiceTest.ktbackend/app/src/test/resources/junit-platform.propertiesbackend/testing/build.gradlebackend/testing/src/main/kotlin/io/tolgee/AbstractSpringTest.ktbackend/testing/src/main/kotlin/io/tolgee/CleanDbTestListener.ktbackend/testing/src/main/kotlin/io/tolgee/config/TestPostHogConfiguration.ktbackend/testing/src/main/kotlin/io/tolgee/testing/TestShardCondition.ktbackend/testing/src/main/resources/META-INF/services/org.junit.jupiter.api.extension.Extension
💤 Files with no reviewable changes (15)
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerInvitationTest.kt
- backend/app/src/test/kotlin/io/tolgee/activity/ActivityLogTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/BusinessEventControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/UserMfaControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/service/notification/NotificationServiceTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/organizationController/OrganizationControllerInvitingTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2UserControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/security/EmailVerificationTest.kt
- backend/app/src/test/kotlin/io/tolgee/controllers/PublicControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/controllers/resetPassword/ResetPasswordControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ExportController/V2ExportControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/mcp/McpServerIntegrationTest.kt
- backend/app/src/test/kotlin/io/tolgee/controllers/MarketingEmailingTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/notification/NotificationSettingsControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2InvitationControllerTest.kt
393607f to
cbb211c
Compare
… context recreation @import(TestEmailConfiguration) was declared on 10 individual test classes, causing each to create a separate Spring context. Moving it to AbstractSpringTest lets all tests share the base context, eliminating ~2 unnecessary context startups. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ostHog contexts Replace per-test @MockitoBean PostHog with a shared @TestConfiguration that provides a @bean @primary mock. This removes PostHog as a context-splitting factor across 5 test classes, reducing the number of Spring context recreations during test runs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…anchAutopushTest Add @MockitoBean EnabledFeaturesProvider to AutomationIntegrationTest so both content delivery tests share the same Spring context instead of each creating their own. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Align KeyServiceTest and LanguageServiceTest property sets with OrganizationServiceTest so all three share a single Spring context instead of two. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ats tests Align property set so it shares a Spring context with KeyServiceTest, LanguageServiceTest, and OrganizationServiceTest. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…able filtering - Use TRUNCATE CASCADE instead of manually disabling/enabling triggers on all 109 tables (eliminates 218 ALTER TABLE statements per clean) - Only truncate tables that actually contain data, detected via a single UNION ALL EXISTS query - Cache the table list from information_schema (static during test run) Before: ~243ms avg per clean, 1010 cleans = ~246s total After: skips empty tables entirely, no trigger toggling overhead Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add a JUnit 5 ExecutionCondition that distributes test classes across shards based on class name hash. Controlled via SHARD_INDEX/SHARD_TOTAL env vars — when not set, all tests run (backward compatible). CI workflow now runs two parallel jobs for runStandardTests, cutting wall time from ~11m to ~6m. Locally verified: shard 0 = 3m40s, shard 1 = 3m46s (well balanced). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ition - Add @volatile to nonEmptyCheckQuery for thread-safe visibility - Fix Int.MIN_VALUE edge case in shard hash: use bitwise AND instead of absoluteValue Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
f76d20c to
8628aca
Compare
Use the server-side closedAt from the cancel response instead of client-side System.currentTimeMillis() to avoid clock skew between the test and the server. Also remove assertion on task ordering (tasks[0].name) which is non-deterministic. Without fix: 2/1000 failures. With fix: 881/881 passes.
989e9db to
deba898
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/testing/src/main/kotlin/io/tolgee/CleanDbTestListener.kt`:
- Around line 112-117: In CleanDbTestListener where the cleanup SQL is built
(the stmt.executeQuery call that selects table_schema and table_name), remove
the hardcoded "table_catalog = 'postgres'" predicate and only filter by schema
(table_schema in ('public','billing','ee')) and table_name not in
($ignoredTablesString); use the existing DataSource connection (as you're
already connected to the target DB) so the catalog filter isn't needed and will
break when postgresAutostartProperties.databaseName is changed.
In `@backend/testing/src/main/kotlin/io/tolgee/testing/TestShardCondition.kt`:
- Around line 22-28: The parsed shardIndex value is not validated against
shardTotal, so add a range check after parsing in TestShardCondition (where
shardIndex and shardTotal are used): if shardIndex is null we already enable,
but when parsed ensure shardIndex >= 0 && shardIndex < shardTotal and if not
return ConditionEvaluationResult.disabled(...) with a clear message (e.g.
"Invalid shard index X for total Y") so misconfigured indices don't cause
misleading green runs; update the branch that currently uses shardIndex to rely
on this validated value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 836eaf78-60da-4a3a-b21a-88aa983bc93e
📒 Files selected for processing (29)
.github/workflows/test.ymlbackend/app/build.gradlebackend/app/src/test/kotlin/io/tolgee/activity/ActivityLogTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/BusinessEventControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/UserMfaControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2InvitationControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2UserControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/notification/NotificationSettingsControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/organizationController/OrganizationControllerInvitingTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ExportController/V2ExportControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerInvitationTest.ktbackend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.ktbackend/app/src/test/kotlin/io/tolgee/controllers/MarketingEmailingTest.ktbackend/app/src/test/kotlin/io/tolgee/controllers/PublicControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/controllers/resetPassword/ResetPasswordControllerTest.ktbackend/app/src/test/kotlin/io/tolgee/mcp/McpServerIntegrationTest.ktbackend/app/src/test/kotlin/io/tolgee/security/EmailVerificationTest.ktbackend/app/src/test/kotlin/io/tolgee/service/KeyServiceTest.ktbackend/app/src/test/kotlin/io/tolgee/service/LanguageServiceTest.ktbackend/app/src/test/kotlin/io/tolgee/service/UserAccountServiceTest.ktbackend/app/src/test/kotlin/io/tolgee/service/notification/NotificationServiceTest.ktbackend/app/src/test/resources/junit-platform.propertiesbackend/testing/build.gradlebackend/testing/src/main/kotlin/io/tolgee/AbstractSpringTest.ktbackend/testing/src/main/kotlin/io/tolgee/CleanDbTestListener.ktbackend/testing/src/main/kotlin/io/tolgee/config/TestPostHogConfiguration.ktbackend/testing/src/main/kotlin/io/tolgee/testing/TestShardCondition.ktbackend/testing/src/main/resources/META-INF/services/org.junit.jupiter.api.extension.Extensionee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/task/TaskControllerTest.kt
💤 Files with no reviewable changes (15)
- backend/app/src/test/kotlin/io/tolgee/activity/ActivityLogTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2UserControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerInvitationTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/organizationController/OrganizationControllerInvitingTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/BusinessEventControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/controllers/resetPassword/ResetPasswordControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/UserMfaControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/notification/NotificationSettingsControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/controllers/MarketingEmailingTest.kt
- backend/app/src/test/kotlin/io/tolgee/controllers/PublicControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2InvitationControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ExportController/V2ExportControllerTest.kt
- backend/app/src/test/kotlin/io/tolgee/security/EmailVerificationTest.kt
- backend/app/src/test/kotlin/io/tolgee/mcp/McpServerIntegrationTest.kt
- backend/app/src/test/kotlin/io/tolgee/service/notification/NotificationServiceTest.kt
✅ Files skipped from review due to trivial changes (7)
- backend/app/src/test/resources/junit-platform.properties
- backend/testing/build.gradle
- backend/app/src/test/kotlin/io/tolgee/service/KeyServiceTest.kt
- backend/app/src/test/kotlin/io/tolgee/service/UserAccountServiceTest.kt
- backend/app/src/test/kotlin/io/tolgee/service/LanguageServiceTest.kt
- backend/testing/src/main/resources/META-INF/services/org.junit.jupiter.api.extension.Extension
- backend/testing/src/main/kotlin/io/tolgee/config/TestPostHogConfiguration.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.kt
- backend/testing/src/main/kotlin/io/tolgee/AbstractSpringTest.kt
- .github/workflows/test.yml
|
@dkrizan @JanCizmar could you please review |
# [3.180.0](v3.179.5...v3.180.0) (2026-04-15) ### Features * refactor work with spring boot test context ([#3561](#3561)) ([67e06bf](67e06bf))
Summary by CodeRabbit