Skip to content

Commit e4c79f0

Browse files
committed
refactor: introduce Clock service for testable time management
- Create Clock trait abstraction in shared layer - Implement SystemClock using Utc::now() for production - Implement MockClock with time control for tests - Refactor ProvisionCommand to inject Clock dependency - Refactor ConfigureCommand to inject Clock dependency - Update dependency injection container with Clock service - Update all command unit tests to use Clock - Update E2E test tasks to pass Clock from test context - Add comprehensive Clock service section to testing.md - All 735 unit tests pass with new Clock abstraction This makes time-dependent code deterministic and testable by allowing tests to control time progression without actual delays.
1 parent 9686f9d commit e4c79f0

File tree

10 files changed

+506
-13
lines changed

10 files changed

+506
-13
lines changed

docs/contributing/testing.md

Lines changed: 118 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,123 @@ mod tests {
255255
}
256256
```
257257

258+
### Using the Clock Service for Deterministic Time Tests
259+
260+
Time is treated as an infrastructure concern. Always use the `Clock` trait instead of calling `Utc::now()` directly to make time-dependent code testable and deterministic.
261+
262+
#### Why Use Clock Service?
263+
264+
Direct use of `Utc::now()` makes tests:
265+
266+
- **Non-deterministic**: Tests produce different results on each run
267+
- **Hard to test**: Cannot control time progression or test specific timestamps
268+
- **Difficult to debug**: Time-related issues are hard to reproduce
269+
270+
The Clock service solves these problems by:
271+
272+
- **Controlling time in tests**: Set specific timestamps for predictable behavior
273+
- **Making tests deterministic**: Same test always produces same result
274+
- **Testing time-dependent logic**: Simulate time progression without actual delays
275+
- **Enabling edge case testing**: Test timeouts, expiration, and time-based conditions
276+
277+
#### Production Code
278+
279+
In production code, inject the `Clock` dependency:
280+
281+
```rust
282+
use crate::shared::Clock;
283+
use chrono::{DateTime, Utc};
284+
285+
pub struct EventRecorder {
286+
clock: Arc<dyn Clock>,
287+
}
288+
289+
impl EventRecorder {
290+
pub fn new(clock: Arc<dyn Clock>) -> Self {
291+
Self { clock }
292+
}
293+
294+
pub fn record_event(&self) -> DateTime<Utc> {
295+
// Use clock.now() instead of Utc::now()
296+
let timestamp = self.clock.now();
297+
println!("Event recorded at: {}", timestamp);
298+
timestamp
299+
}
300+
}
301+
```
302+
303+
#### Test Code
304+
305+
In tests, use `MockClock` for full control over time:
306+
307+
```rust
308+
use crate::testing::MockClock;
309+
use chrono::{TimeZone, Utc};
310+
use std::sync::Arc;
311+
312+
#[test]
313+
fn it_should_record_event_with_specific_timestamp() {
314+
// Arrange: Set up mock clock with fixed time
315+
let fixed_time = Utc.with_ymd_and_hms(2025, 10, 7, 12, 0, 0).unwrap();
316+
let clock = Arc::new(MockClock::new(fixed_time));
317+
let recorder = EventRecorder::new(clock.clone());
318+
319+
// Act: Record event
320+
let recorded_time = recorder.record_event();
321+
322+
// Assert: Verify exact timestamp
323+
assert_eq!(recorded_time, fixed_time);
324+
}
325+
326+
#[test]
327+
fn it_should_handle_time_progression() {
328+
// Arrange: Set up mock clock
329+
let start_time = Utc.with_ymd_and_hms(2025, 10, 7, 12, 0, 0).unwrap();
330+
let clock = Arc::new(MockClock::new(start_time));
331+
let recorder = EventRecorder::new(clock.clone());
332+
333+
// Act: Record first event
334+
let first_event = recorder.record_event();
335+
336+
// Simulate 5 minutes passing
337+
clock.advance_secs(300);
338+
339+
// Record second event
340+
let second_event = recorder.record_event();
341+
342+
// Assert: Verify time difference
343+
let expected_second = Utc.with_ymd_and_hms(2025, 10, 7, 12, 5, 0).unwrap();
344+
assert_eq!(first_event, start_time);
345+
assert_eq!(second_event, expected_second);
346+
}
347+
```
348+
349+
#### Key Benefits
350+
351+
- **Deterministic Tests**: Tests always produce the same results
352+
- **Fast Execution**: No need for actual time delays with `sleep()`
353+
- **Edge Case Testing**: Easily test timeouts, expirations, and time boundaries
354+
- **Improved Debugging**: Failures are reproducible with exact timestamps
355+
- **Better Test Coverage**: Can test time-dependent scenarios that would be impractical otherwise
356+
357+
#### When to Use
358+
359+
Use `MockClock` when testing:
360+
361+
- Timestamp generation and recording
362+
- Timeout and expiration logic
363+
- Time-based retries and backoff strategies
364+
- Duration calculations and measurements
365+
- Time-series data processing
366+
- Scheduled operations and cron-like behavior
367+
258368
## 🚀 Getting Started
259369

260-
When writing new tests, always use the `it_should_` prefix and describe the specific behavior being validated. This makes the test suite more readable and maintainable for all contributors.
370+
When writing new tests:
371+
372+
- Always use the `it_should_` prefix and describe the specific behavior being validated
373+
- Use `MockClock` for any time-dependent tests instead of `Utc::now()`
374+
- Follow the AAA pattern for clear test structure
375+
- Ensure tests are isolated and don't interfere with each other
376+
377+
This makes the test suite more readable, maintainable, and reliable for all contributors.

src/application/commands/configure.rs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ impl crate::shared::Traceable for ConfigureCommandError {
5757
/// Persistence failures are logged but don't fail the command (state remains valid in memory).
5858
pub struct ConfigureCommand {
5959
ansible_client: Arc<AnsibleClient>,
60+
clock: Arc<dyn crate::shared::Clock>,
6061
repository: Arc<dyn EnvironmentRepository>,
6162
}
6263

@@ -65,10 +66,12 @@ impl ConfigureCommand {
6566
#[must_use]
6667
pub fn new(
6768
ansible_client: Arc<AnsibleClient>,
69+
clock: Arc<dyn crate::shared::Clock>,
6870
repository: Arc<dyn EnvironmentRepository>,
6971
) -> Self {
7072
Self {
7173
ansible_client,
74+
clock,
7275
repository,
7376
}
7477
}
@@ -223,7 +226,6 @@ impl ConfigureCommand {
223226
environment: &Environment<Configuring>,
224227
error: &ConfigureCommandError,
225228
) -> ConfigureFailureContext {
226-
use chrono::Utc;
227229
use std::time::Duration;
228230

229231
let (failed_step, error_kind) = match error {
@@ -238,7 +240,7 @@ impl ConfigureCommand {
238240
}
239241
};
240242

241-
let now = Utc::now();
243+
let now = self.clock.now();
242244
let trace_id = TraceId::new();
243245

244246
let mut context = ConfigureFailureContext {
@@ -287,18 +289,26 @@ mod tests {
287289

288290
// Helper function to create mock dependencies for testing
289291
#[allow(clippy::type_complexity)]
290-
fn create_mock_dependencies() -> (Arc<AnsibleClient>, Arc<dyn EnvironmentRepository>, TempDir) {
292+
fn create_mock_dependencies() -> (
293+
Arc<AnsibleClient>,
294+
Arc<dyn crate::shared::Clock>,
295+
Arc<dyn EnvironmentRepository>,
296+
TempDir,
297+
) {
291298
let temp_dir = TempDir::new().expect("Failed to create temp dir");
292299
let ansible_client = Arc::new(AnsibleClient::new(temp_dir.path()));
293300

301+
// Create clock
302+
let clock: Arc<dyn crate::shared::Clock> = Arc::new(crate::shared::SystemClock);
303+
294304
// Create repository
295305
let repository_factory =
296306
crate::infrastructure::persistence::repository_factory::RepositoryFactory::new(
297307
std::time::Duration::from_secs(30),
298308
);
299309
let repository = repository_factory.create(temp_dir.path().to_path_buf());
300310

301-
(ansible_client, repository, temp_dir)
311+
(ansible_client, clock, repository, temp_dir)
302312
}
303313

304314
// Helper function to create a test environment in Configuring state
@@ -321,9 +331,9 @@ mod tests {
321331

322332
#[test]
323333
fn it_should_create_configure_command_with_all_dependencies() {
324-
let (ansible_client, repository, _temp_dir) = create_mock_dependencies();
334+
let (ansible_client, clock, repository, _temp_dir) = create_mock_dependencies();
325335

326-
let command = ConfigureCommand::new(ansible_client, repository);
336+
let command = ConfigureCommand::new(ansible_client, clock, repository);
327337

328338
// Verify the command was created (basic structure test)
329339
// This test just verifies that the command can be created with the dependencies
@@ -343,9 +353,9 @@ mod tests {
343353

344354
#[test]
345355
fn it_should_build_failure_context_from_command_error() {
346-
let (ansible_client, repository, temp_dir) = create_mock_dependencies();
356+
let (ansible_client, clock, repository, temp_dir) = create_mock_dependencies();
347357

348-
let command = ConfigureCommand::new(ansible_client, repository);
358+
let command = ConfigureCommand::new(ansible_client, clock, repository);
349359

350360
// Create test environment for trace generation
351361
let (environment, _env_temp_dir) = create_test_environment(&temp_dir);

src/application/commands/provision.rs

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ use crate::domain::environment::repository::EnvironmentRepository;
2727
use crate::domain::environment::state::{
2828
ProvisionErrorKind, ProvisionFailureContext, ProvisionStep,
2929
};
30-
use crate::domain::environment::{TraceId,
31-
Created, Environment, ProvisionFailed, Provisioned, Provisioning,
30+
use crate::domain::environment::{
31+
Created, Environment, ProvisionFailed, Provisioned, Provisioning, TraceId,
3232
};
3333
#[allow(unused_imports)]
3434
use crate::domain::{InstanceName, ProfileName};
@@ -121,6 +121,7 @@ pub struct ProvisionCommand {
121121
ansible_client: Arc<AnsibleClient>,
122122
opentofu_client:
123123
Arc<crate::infrastructure::external_tools::tofu::adapter::client::OpenTofuClient>,
124+
clock: Arc<dyn crate::shared::Clock>,
124125
repository: Arc<dyn EnvironmentRepository>,
125126
}
126127

@@ -134,13 +135,15 @@ impl ProvisionCommand {
134135
opentofu_client: Arc<
135136
crate::infrastructure::external_tools::tofu::adapter::client::OpenTofuClient,
136137
>,
138+
clock: Arc<dyn crate::shared::Clock>,
137139
repository: Arc<dyn EnvironmentRepository>,
138140
) -> Self {
139141
Self {
140142
tofu_template_renderer,
141143
ansible_template_renderer,
142144
ansible_client,
143145
opentofu_client,
146+
clock,
144147
repository,
145148
}
146149
}
@@ -410,7 +413,6 @@ impl ProvisionCommand {
410413
error: &ProvisionCommandError,
411414
) -> ProvisionFailureContext {
412415
use crate::infrastructure::trace::ProvisionTraceWriter;
413-
use chrono::Utc;
414416
use std::time::Duration;
415417

416418
let (failed_step, error_kind) = match error {
@@ -439,7 +441,7 @@ impl ProvisionCommand {
439441
),
440442
};
441443

442-
let now = Utc::now();
444+
let now = self.clock.now();
443445
let trace_id = TraceId::new();
444446

445447
// Build initial context without trace file path
@@ -511,6 +513,7 @@ mod tests {
511513
Arc<AnsibleTemplateRenderer>,
512514
Arc<AnsibleClient>,
513515
Arc<crate::infrastructure::external_tools::tofu::adapter::client::OpenTofuClient>,
516+
Arc<dyn crate::shared::Clock>,
514517
Arc<dyn EnvironmentRepository>,
515518
SshCredentials,
516519
TempDir,
@@ -563,11 +566,15 @@ mod tests {
563566
Username::new("test_user").unwrap(),
564567
);
565568

569+
// Create mock clock for testing
570+
let clock: Arc<dyn crate::shared::Clock> = Arc::new(crate::shared::SystemClock);
571+
566572
(
567573
tofu_renderer,
568574
ansible_renderer,
569575
ansible_client,
570576
opentofu_client,
577+
clock,
571578
repository,
572579
ssh_credentials,
573580
temp_dir,
@@ -594,6 +601,7 @@ mod tests {
594601
ansible_renderer,
595602
ansible_client,
596603
opentofu_client,
604+
clock,
597605
repository,
598606
_ssh_credentials,
599607
_temp_dir,
@@ -604,6 +612,7 @@ mod tests {
604612
ansible_renderer,
605613
ansible_client,
606614
opentofu_client,
615+
clock,
607616
repository,
608617
);
609618

@@ -656,6 +665,7 @@ mod tests {
656665
ansible_renderer,
657666
ansible_client,
658667
opentofu_client,
668+
clock,
659669
repository,
660670
_ssh_credentials,
661671
temp_dir,
@@ -666,6 +676,7 @@ mod tests {
666676
ansible_renderer,
667677
ansible_client,
668678
opentofu_client,
679+
clock,
669680
repository,
670681
);
671682

@@ -695,6 +706,7 @@ mod tests {
695706
ansible_renderer,
696707
ansible_client,
697708
opentofu_client,
709+
clock,
698710
repository,
699711
_ssh_credentials,
700712
temp_dir,
@@ -705,6 +717,7 @@ mod tests {
705717
ansible_renderer,
706718
ansible_client,
707719
opentofu_client,
720+
clock,
708721
repository,
709722
);
710723

@@ -728,6 +741,7 @@ mod tests {
728741
ansible_renderer,
729742
ansible_client,
730743
opentofu_client,
744+
clock,
731745
repository,
732746
_ssh_credentials,
733747
temp_dir,
@@ -738,6 +752,7 @@ mod tests {
738752
ansible_renderer,
739753
ansible_client,
740754
opentofu_client,
755+
clock,
741756
repository,
742757
);
743758

@@ -762,6 +777,7 @@ mod tests {
762777
ansible_renderer,
763778
ansible_client,
764779
opentofu_client,
780+
clock,
765781
repository,
766782
_ssh_credentials,
767783
temp_dir,
@@ -772,6 +788,7 @@ mod tests {
772788
ansible_renderer,
773789
ansible_client,
774790
opentofu_client,
791+
clock,
775792
repository,
776793
);
777794

0 commit comments

Comments
 (0)