UNOMI-881: Add in-memory test harness and service unit tests (part 1/2)#772
Merged
Conversation
Introduce the reusable in-memory persistence test framework and independent service unit tests from unomi-3-dev, adapted for master without tracing or validation dependencies. Adds minimal persistence-spi hooks for test wiring.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR backports an in-memory persistence test harness onto master and introduces an initial batch of service-layer unit tests, with minimal SPI changes to make the harness compile and run without tracing/validation modules.
Changes:
- Add a test harness (
TestHelper, OSGi/test doubles) and a large set of service unit tests underservices. - Extend persistence SPI with evaluator registration hooks and a
DefinitionsServicesetter, plus broadenDateUtilsdate-type support. - Add test resources for predefined properties/personas/segments and test-only dependencies needed by the harness.
Reviewed changes
Copilot reviewed 23 out of 26 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| services/src/test/resources/test-segments/test-segment.json | Adds a segment JSON fixture used by service tests. |
| services/src/test/resources/META-INF/cxs/properties/predefined-properties.json | Adds a predefined property JSON fixture used by profile/property loading tests. |
| services/src/test/resources/META-INF/cxs/personas/predefined-personas.json | Adds a predefined persona JSON fixture used by persona loading tests. |
| services/src/test/java/org/apache/unomi/services/TestHelper.java | Introduces shared wiring/teardown helpers for in-memory tests. |
| services/src/test/java/org/apache/unomi/services/impl/TestTenantServiceTest.java | Adds unit tests for the test tenant service API-key behavior. |
| services/src/test/java/org/apache/unomi/services/impl/TestTenantService.java | Adds an in-memory TenantService implementation for tests. |
| services/src/test/java/org/apache/unomi/services/impl/TestRequestTracer.java | Adds a no-op tracer to decouple tests from tracing modules. |
| services/src/test/java/org/apache/unomi/services/impl/TestEventAdmin.java | Adds an OSGi EventAdmin test double for async/sync event delivery. |
| services/src/test/java/org/apache/unomi/services/impl/TestConditionEvaluators.java | Adds condition evaluator implementations used by persistence/test harness. |
| services/src/test/java/org/apache/unomi/services/impl/TestBundleContext.java | Adds a simplified BundleContext implementation for tests. |
| services/src/test/java/org/apache/unomi/services/impl/SimpleItem.java | Adds a simple Item subtype used in evaluator/persistence tests. |
| services/src/test/java/org/apache/unomi/services/impl/scheduler/TaskExecutorRegistryTest.java | Adds unit tests for scheduler executor registry behavior. |
| services/src/test/java/org/apache/unomi/services/impl/rules/TestSetEventOccurrenceCountAction.java | Adds a test-only copy of an action executor for rules-related tests. |
| services/src/test/java/org/apache/unomi/services/impl/rules/TestEvaluateProfileSegmentsAction.java | Adds a test-only action executor for segment evaluation tests. |
| services/src/test/java/org/apache/unomi/services/impl/profiles/ProfileServiceImplTest.java | Adds unit tests for profile service property/persona behavior using fixtures. |
| services/src/test/java/org/apache/unomi/services/impl/NestedItem.java | Adds a nested Item subtype used by nested-condition tests. |
| services/src/test/java/org/apache/unomi/services/impl/ExecutionContextManagerImplTest.java | Adds unit tests for execution context role/permission aggregation. |
| services/src/test/java/org/apache/unomi/services/impl/cluster/ClusterServiceImplTest.java | Adds unit tests for cluster service persistence and scheduling behavior. |
| services/src/test/java/org/apache/unomi/services/impl/cache/MultiTypeCacheServiceImplTest.java | Adds unit tests for multi-tenant cache behavior and statistics. |
| services/pom.xml | Adds commons-io as a test dependency for test harness cleanup. |
| persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/evaluator/impl/ConditionEvaluatorDispatcherImpl.java | Adds minimal hooks for setting definitions service and manual evaluator registration. |
| persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/evaluator/ConditionEvaluatorDispatcher.java | Extends dispatcher API with addEvaluator / removeEvaluator. |
| persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/DateUtils.java | Adds support for modern java.time types when converting to java.util.Date. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+125
to
+129
| this.asyncExecutor = Executors.newSingleThreadExecutor(r -> { | ||
| Thread t = new Thread(r, "TestEventAdmin-Async-" + System.identityHashCode(this)); | ||
| t.setDaemon(true); | ||
| return t; | ||
| }); |
Comment on lines
+422
to
+426
| // Wait for all handler queues to be empty | ||
| for (BlockingQueue<Event> queue : handlerQueues.values()) { | ||
| while (!queue.isEmpty() && System.currentTimeMillis() < deadline) { | ||
| try { | ||
| Thread.sleep(10); |
Comment on lines
+83
to
85
| if (value instanceof LocalDateTime) { | ||
| return Date.from(((LocalDateTime) value).atZone(ZoneId.systemDefault()).toInstant()); | ||
| } else { |
Comment on lines
+33
to
+36
| /** | ||
| * Test implementation of the SetEventOccurrenceCountAction. | ||
| * @TODO This is a duplicate of the SetEventOccurrenceCountAction in the unomi-plugins-base project to avoid introducing a depending to it but should be cleaned up. | ||
| */ |
Comment on lines
+421
to
+425
| public void testMultipleTypeConfigurations() { | ||
| // First configuration | ||
| CacheableTypeConfig<TestSerializable> config1 = CacheableTypeConfig.<TestSerializable>builder( | ||
| TestSerializable.class, | ||
| TEST_TYPE, |
- TestEventAdmin: replace single-thread executor with cached pool so all handler workers can run concurrently (single thread was blocked by the first handler's queue.take()) - TestEventAdmin: fix waitForEventProcessing race by tracking in-flight events; queue.isEmpty() returned early before handleEvent completed - DateUtils: use ZoneOffset.UTC for LocalDateTime conversion instead of systemDefault() to ensure machine-independent, UTC-consistent behaviour - TestSetEventOccurrenceCountAction: fix Javadoc typo and @todo tag - MultiTypeCacheServiceImplTest: fix testMultipleTypeConfigurations to use two distinct Java types so configs don't overwrite each other, and assert actual cache isolation
- Fix isItemAvailableForQuery to check pendingRefreshItems before refreshedIndexes so FALSE-policy saves correctly defer per-item visibility without hiding other items in the same index - Disable refresh delay simulation in ClusterServiceImplTest (cluster tests verify cluster logic, not persistence refresh semantics) - Fix testLockStealing: revert retryUntil to conditional check since completed tasks have null lockOwner - Fix testGetClusterNodesReturnsAllNodes: use direct reflection call to updateSystemStats() instead of retryUntil (cache is updated every 10 s, far longer than the polling budget) - Replace slf4j-simple with logback-classic for test scope to enable ListAppender support - Add logback-test.xml with INFO default level; override via -DTEST_LOG_LEVEL=<level> - Use ListAppender in shouldHandleTransformationErrorsGracefully to suppress expected stack traces from console while asserting WARNs are actually logged - Fix purge(null) to reset all state (itemsById, pendingRefreshItems, refreshedIndexes) for proper test isolation - Fix various silent failure, type safety and API contract issues identified in multi-agent PR review
4 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
unomi-3-devand land the first batch of independent service unit tests (300 tests inservices).persistence-spihooks (addEvaluator/removeEvaluator,setDefinitionsService) and modern date-type handling inDateUtilsso the harness compiles and runs onmasterwithout tracing or validation modules.TestRequestTracer, stripped validation/tracing factories fromTestHelper, explicit crash simulation in scheduler recovery test.Part 1/2 of UNOMI-881. Tests that depend on tracing (UNOMI-873) or condition validation (UNOMI-883) are deferred to a follow-up PR.
Test plan
mvn -pl services -am test -Dmaven.build.cache.enabled=false -Dsurefire.failIfNoSpecifiedTests=false