Skip to content

UNOMI-881: Add service and extension unit tests (part 2/2)#776

Merged
sergehuber merged 3 commits into
masterfrom
UNOMI-881-service-tests
Jun 18, 2026
Merged

UNOMI-881: Add service and extension unit tests (part 2/2)#776
sergehuber merged 3 commits into
masterfrom
UNOMI-881-service-tests

Conversation

@sergehuber

@sergehuber sergehuber commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds UNOMI-881 part 2: service unit tests that depend on tracing (UNOMI-873) and condition validation/type resolution (UNOMI-883), deferred from UNOMI-881: Add in-memory test harness and service unit tests (part 1/2) #772.
  • Extends the test harness (TestHelper, TestRequestTracer, TestActionExecutorDispatcher, etc.) with tracing-aware factories for rules, events, and definitions.
  • Adds extension unit tests for GroovyActionsServiceImpl and SchemaServiceImpl with test resources.

Test plan

  • mvn -pl services install -DskipTests (refresh test-jar for extension modules)
  • mvn -pl services,extensions/groovy-actions/services,extensions/json-schema/services test
  • mvn -pl services test (full services suite, ~547 tests)
  • CI green

Notes

  • Test-only backport from unomi-3-dev; no production code changes.
  • Master review fixes preserved (clearCurrentTenantId, cleanupReferences no-op, TestTracerService.cleanup()).
  • Completes the planned backport stack (PR E of A→E).

@sergehuber
UNOMI-881: Add service and extension unit tests (part 2/2)
e80c835 
Land tracing- and validation-dependent unit tests deferred from PR B, plus harness helpers for rules/events/tracing and extension test coverage for groovy-actions and json-schema.
@sergehuber sergehuber requested a review from Copilot June 18, 2026 05:33
Copilot AI reviewed Jun 18, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds the second batch of UNOMI-881 unit tests and expands the in-memory test harness to support tracing- and validation-dependent services, including new coverage for segments, goals, events, Groovy actions, and JSON schema handling.

Changes:

  • Extend the shared test harness (TestHelper, TestRequestTracer, TestEventAdmin, condition evaluators) to wire tracing and additional service factories.
  • Add new service-level unit tests for segments, goals, events, and action dispatching.
  • Add extension unit tests and test resources for JSON schema and Groovy actions modules.

Reviewed changes

Copilot reviewed 52 out of 53 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
services/src/test/java/org/apache/unomi/services/TestHelper.java Adds tracing-aware service factories and refines retry/teardown utilities.
services/src/test/java/org/apache/unomi/services/impl/TestRequestTracer.java Switches test tracer to extend LoggingRequestTracer and collect trace output.
services/src/test/java/org/apache/unomi/services/impl/TestEventAdmin.java Updates async delivery model and event-processing wait helper.
services/src/test/java/org/apache/unomi/services/impl/TestConditionEvaluators.java Updates evaluators and date handling used by the in-memory harness tests.
services/src/test/java/org/apache/unomi/services/impl/TestBundleContext.java Adjusts mock OSGi bundle context behavior.
services/src/test/java/org/apache/unomi/services/impl/segments/SegmentServiceImplTest.java Adds extensive SegmentService unit tests using the in-memory harness.
services/src/test/java/org/apache/unomi/services/impl/rules/TestSetEventOccurrenceCountAction.java Tweaks test action implementation documentation.
services/src/test/java/org/apache/unomi/services/impl/rules/TestActionExecutorDispatcher.java Introduces a test dispatcher to execute action executors with optional tracing.
services/src/test/java/org/apache/unomi/services/impl/InMemoryPersistenceServiceImplTest.java Improves retry logic in a couple of pagination-related assertions.
services/src/test/java/org/apache/unomi/services/impl/goals/GoalsServiceImplTest.java Adds GoalsService unit tests that depend on validation/tracing wiring.
services/src/test/java/org/apache/unomi/services/impl/events/EventServiceImplTest.java Adds EventService unit tests (send/search/authorization edge cases).
services/src/test/java/org/apache/unomi/services/actions/impl/ActionExecutorDispatcherImplTest.java Adds focused tests for action type resolution before dispatch.
extensions/json-schema/services/src/test/java/org/apache/unomi/schema/impl/SchemaServiceImplTest.java Adds SchemaService unit tests for tenant isolation, inheritance, merge, and validation.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/view-event-schema.json Adds base schema fixture for extension/merge tests.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/validation-schema.json Adds validation schema fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/updated-schema.json Adds updated schema fixture for dynamic update tests.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/test-scope-schema.json Adds target/scope fixture schema.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/tenant2-extension.json Adds tenant2 extension fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/tenant1-schema.json Adds tenant1 schema fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/tenant1-extension.json Adds tenant1 extension fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/tenant-specific-schema.json Adds tenant-only schema fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/tenant-override-schema.json Adds tenant override fixture for same $id.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/tenant-event-schema.json Adds tenant event schema fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/target-schema6.json Adds target-schema fixture (tenant2).
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/target-schema5.json Adds target-schema fixture (different target).
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/target-schema4.json Adds target-schema fixture (tenant1 #2).
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/target-schema3.json Adds target-schema fixture (tenant1 #1).
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/target-schema2.json Adds target-schema fixture (system #2).
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/target-schema1.json Adds target-schema fixture (system #1).
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/system-schema.json Adds system schema fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/system-override-schema.json Adds system override fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/system-inheritance-schema.json Adds system inheritance fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/system-extension.json Adds system extension fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/system-event-schema.json Adds system event schema fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/schema2.json Adds tenant schema fixture sharing $id with system.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/schema1.json Adds system schema fixture sharing $id with tenant.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/predefined-schemas.json Adds predefined schema fixture loaded via bundle scanning.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/merge-tenant-only.json Adds tenant-only merge fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/merge-system-only.json Adds system-only merge fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/merge-shared-schema-tenant.json Adds tenant override for merge fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/merge-shared-schema-system.json Adds system base for merge fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/initial-schema.json Adds initial dynamic schema fixture.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/autodetect-tenant-event-schema.json Adds tenant event schema for auto-detect validation tests.
extensions/json-schema/services/src/test/resources/META-INF/cxs/schemas/autodetect-system-event-schema.json Adds system event schema for auto-detect validation tests.
extensions/groovy-actions/services/src/test/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImplTest.java Adds GroovyActionsService unit tests (save/remove/execute/isolation).
extensions/groovy-actions/services/src/test/resources/META-INF/cxs/actions/testSaveAction.groovy Adds Groovy script fixture for save tests.
extensions/groovy-actions/services/src/test/resources/META-INF/cxs/actions/testRemoveAction.groovy Adds Groovy script fixture for remove tests.
extensions/groovy-actions/services/src/test/resources/META-INF/cxs/actions/testExecuteAction.groovy Adds Groovy script fixture for execution/parameter tests.
extensions/groovy-actions/services/src/test/resources/META-INF/cxs/actions/testAction.groovy Adds general Groovy action script fixture.
extensions/groovy-actions/services/src/test/resources/META-INF/cxs/actions/predefined-actions.json Adds predefined-actions fixture for bundle scanning tests.
extensions/groovy-actions/services/src/test/resources/META-INF/base/BaseScript.groovy Adds base script fixture used by Groovy action execution.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread services/src/test/java/org/apache/unomi/services/impl/TestEventAdmin.java Outdated
Comment thread services/src/test/java/org/apache/unomi/services/impl/TestEventAdmin.java Outdated
Comment thread services/src/test/java/org/apache/unomi/services/impl/TestConditionEvaluators.java
Comment thread services/src/test/java/org/apache/unomi/services/impl/TestConditionEvaluators.java
Comment thread services/src/test/java/org/apache/unomi/services/impl/TestConditionEvaluators.java
Comment thread services/src/test/java/org/apache/unomi/services/impl/TestBundleContext.java
Comment thread services/src/test/java/org/apache/unomi/services/TestHelper.java
Comment thread services/src/test/java/org/apache/unomi/services/TestHelper.java
Comment thread services/src/test/java/org/apache/unomi/services/impl/segments/SegmentServiceImplTest.java Outdated
Comment thread ...es/src/test/java/org/apache/unomi/services/impl/rules/TestSetEventOccurrenceCountAction.java
@sergehuber
UNOMI-881: Fix issues flagged by Copilot review on PR #776
25e6684 
- TestEventAdmin: use cached thread pool instead of single-threaded so
  each handler worker gets its own thread (single thread blocked on first
  queue.take(), starving all subsequent handlers); restore inFlightCount
  tracking in waitForEventProcessing so it waits for handleEvent to
  actually return, not just for the queue to drain
- TestConditionEvaluators: restore instanceof guard before casting
  subConditions; restore null checks in evaluateDateCondition; restore
  instanceof String guards in inContains; fix pastEvents cast from
  ArrayList to List with instanceof check; return unmodifiable map from
  getConditionTypes()
- TestBundleContext: return Collections.emptyList() instead of null from
  getServiceReferences(Class, String)
- TestHelper: restore full exception in warn log; throw RuntimeException
  with cause on InterruptedException in retryQueryUntilAvailable
- SegmentServiceImplTest: remove duplicate evaluateProfileSegments
  executor registration
- TestSetEventOccurrenceCountAction: fix @todo tag and typo in comment
@sergehuber
UNOMI-881: Fix test quality issues found during PR #776 review
5dac72e 
Resource leaks
- Add @after teardown to GroovyActionsServiceImplTest and SchemaServiceImplTest
  (JUnit 4) calling service.preDestroy() and schedulerService.preDestroy(), preventing
  thread-pool accumulation across test methods
- Add @AfterEach to DefinitionsServiceImplTest calling the same, fixing the same leak
  for the JUnit 5 suite
- Add schedulerService.preDestroy() to RulesServiceImplTest teardown (was missing)

Test isolation
- SegmentServiceImplTest: clear TestConditionEvaluators.setEventService(null) in
  tearDown to prevent stale cross-test references
- SegmentServiceImplTest: remove redundant rulesService.postConstruct() call that
  registered duplicate background tasks per test run
- DefinitionsServiceImplTest: remove misleading synchronized(tenantService) wrapper
  that provided no thread safety; CyclicBarrier already handles synchronisation

Correctness
- RulesServiceImplTest: replace assertNotNull(int) (always true due to autoboxing)
  with assertEquals(EventService.XXX, result) throughout loop-detection and
  depth-tracking tests
- RulesServiceImplTest: rewrite testRuleExecutionWithValidCondition (had no
  assertions) as testRuleExecutionWithMatchingCondition with real rule setup and
  return-value assertion
- RulesServiceImplTest: tighten testDepthTrackingAccuracy from assertTrue(count > 0)
  to assertEquals(5, count)
- RulesServiceImplTest: tighten testHistoryTrackingWithMultipleRules to assert
  RuleStatistics execution counts instead of just matching-rule set size
- SegmentServiceImplTest: reload profile from persistence before asserting segment
  membership (was asserting stale in-memory copy)
- SchemaServiceImplTest: replace dual-path disjunction assertions in
  testValidateEvents_BatchValidation with exact key-set equality checks
- GoalsServiceImplTest: remove dead ValidationError list construction that was built
  but never passed to the service under test

Stack traces and exception handling
- GroovyActionsServiceImplTest: replace fail(msg)+return in catch blocks with
  throw new AssertionError(msg, cause) to preserve stack traces and eliminate
  dead code after the unconditional throw
- GroovyActionsServiceImplTest: extract loadGroovyScript() helper that declares
  throws Exception, removing all try/catch boilerplate from test methods
- GroovyActionsServiceImplTest: fix new String(Files.readAllBytes(...)) using
  platform default charset; now uses Files.readString(path, UTF_8)
- RulesServiceImplTest: cast-free teardown and setUp by declaring eventService as
  EventServiceImpl and schedulerService as SchedulerServiceImpl (concrete types
  already used everywhere else in the class)

No Thread.sleep in tests
- DefinitionsServiceImplTest RefreshTimerTests: replace Thread.sleep(150) with
  TestHelper.retryUntil polling the condition type (shouldScheduleAndExecuteRefreshTask,
  shouldHandleRefreshFailureGracefully) or remove outright where no condition exists
  (shouldStopRefreshOnShutdown)
- SchedulerServiceImplTest: replace sleep(100) wait-for-dependent-task with
  retryUntil(dependentExecuted::get, e -> e)
- SchedulerServiceImplTest: replace sleep(TEST_LOCK_TIMEOUT*2) with a retryUntil
  loop that calls recoverCrashedTasks() until taskRecovered is true, eliminating
  the fixed 2-second wait and making recovery detection immediate
- TypeResolutionServiceImplTest: remove Thread.sleep(10) between timestamp
  captures entirely — assertions use >= so no enforced delay is needed

Misc
- InMemoryPersistenceServiceImpl: fix DEFAULT_STORAGE_DIR from "data/persistence"
  (project root, not gitignored) to "target/persistence-data" (Maven output dir)
- TestHelper: remove dead always-true null guard around setPersistenceProvider
- TestConditionEvaluators: add @SuppressWarnings("unchecked") to getValueSet
- SegmentServiceImplTest: initialise requestTracer field (was null, silently
  skipping all tracer-path coverage)
@sergehuber sergehuber merged commit 5ad2314 into master Jun 18, 2026
6 checks passed
@sergehuber sergehuber deleted the UNOMI-881-service-tests branch June 18, 2026 19:52
@sergehuber sergehuber mentioned this pull request Jun 28, 2026
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sergehuber