feat(scheduling): Sync Static Global Clock with Clock Service (Quick Solution) #618
Conversation
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
…clock handling. Add `StaticGlobalClockTest` to ensure correct clock behavior.
dgafka
reviewed
Jan 18, 2026
| $this->assertInstanceOf(Clock::class, $globalClock); | ||
| $this->assertEquals('2025-08-11 16:00:00', $globalClock->now()->format('Y-m-d H:i:s')); | ||
| } | ||
|
|
Member
There was a problem hiding this comment.
Can you provide test case with delay message using new clock strategy?
So similar test to this, but by changing time using clock
Contributor
Author
There was a problem hiding this comment.
Added this separate test: DelayedMessageAgainstGlobalClockTest
dgafka
reviewed
Jan 18, 2026
| return self::$globalClock ??= self::defaultClock(); | ||
| } | ||
|
|
||
| public static function resetGlobalClock(): void |
Member
There was a problem hiding this comment.
Suggested change
| public static function resetGlobalClock(): void | |
| public static function resetToNativeClock(): void |
Contributor
Author
There was a problem hiding this comment.
suggestion applied
- Add `PlaceOrder`, `OrderWasPlaced`, `OrderService`, and `NotificationService` test fixtures. - Improve `Clock` initialization to ensure `globalClock` is only set when unset. - Rename `resetGlobalClock` to `resetToNativeClock` for better clarity. - Add `StaticPsrClock` implementation for static clock testing. - Create `DelayedMessageAgainstGlobalClockTest` to validate delay handling against clock changes. - Update and refactor existing clock-related tests for consistency.
dgafka
approved these changes
Jan 19, 2026
jb-needhelp
commented
Jan 19, 2026
| { | ||
| $ecotoneTestSupport = EcotoneLite::bootstrapFlowTesting( | ||
| [EcotoneClockInterface::class, OrderService::class, NotificationService::class, CustomNotifier::class], | ||
| [$clock = new Clock(new StaticPsrClock('2025-08-11 16:00:00')), new OrderService(), new NotificationService(), $notifier = new CustomNotifier()], |
Contributor
Author
There was a problem hiding this comment.
@dgafka just out of curiosity, can you explain why instance of EcotoneClockInterface on this test is reinstantiated? It's the reason why I added this if condition in Clock::__constructor();
if (!self::$globalClock) {
self::$globalClock = $this->clock ? $this : self::defaultClock();
}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Automatically synchronize
Clock::$globalClockwith the injected clock instance, eliminating the need for manualClock::set()calls in test environments.Why is this change proposed?
Problem
When using a custom clock (e.g., a static clock for testing) with
\Ecotone\Messaging\Scheduling\Clock, the global clock remains unsynchronized. This causes incorrect timestamps in message headers created viaMessageHeaders::createMessageHeadersWith().Currently, the workaround requires calling
Clock::set()explicitly in each test case, which leaks internal implementation details into application tests.Solution
Set
$globalClockautomatically when aClockinstance is constructed:If an internal clock is provided → use it as the global clock
Otherwise → default to NativeClock
This allows applications to define their own
PsrClockInterfaceimplementation without manual global clock management.Cleaner Solution
A cleaner long-term solution would be removing the deprecated
Clock::get()method entirely and replace it withClockinstance. However, this creates a cascade of changes, particularly aroundDelayableQueueChannelwhich callsMessageBuilder::fromMessage($message)without access to aClockinstance.Description of Changes
Ecotone\Messaging\Scheduling\Clock: Auto-set$globalClockon construction; addedresetGlobalClock()for test teardownTest\Ecotone\Messaging\Unit\Scheduling\StaticGlobalClockTest: Verify expected behaviorPull Request Contribution Terms