Skip to content

Conversation

@luismr
Copy link
Owner

@luismr luismr commented May 1, 2025

Pull Request: Add Clock Support for Deterministic Timestamps

Summary

This PR adds configurable clock support to improve testability and consistency of timestamp operations. It introduces a new clock configuration system that allows setting the timezone through application properties while maintaining UTC as the default.

Changes

  • Added ClockConfig and ClockProperties for configurable clock management
  • Modified PingMapper to use injected clock instead of Instant.now()
  • Updated tests to use fixed clocks for deterministic testing
  • Added clock configuration to application.yml
  • Simplified project structure documentation in README.md

Motivation

  • Improve testability by making timestamp operations deterministic
  • Allow timezone configuration through application properties
  • Fix non-deterministic behavior in tests that rely on Instant.now()
  • Make the codebase more maintainable and testable
  • Follow best practices for handling time in Java applications

Checklist

  • Added new configuration classes for clock management
  • Updated mapper to use injected clock
  • Modified tests to use fixed clocks
  • Added configuration documentation
  • Updated README with simplified project structure
  • Added proper JavaDoc comments
  • Ensured backward compatibility with existing code
  • Added appropriate test coverage
  • Followed Java best practices for time handling
  • Used proper dependency injection patterns
  • Maintained consistent code style
  • Added configuration validation
  • Updated documentation to reflect changes
  • Ensured all tests pass
  • Verified timezone configuration works as expected

@luismr luismr requested a review from Copilot May 1, 2025 01:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds configurable clock support to improve testability and deterministic timestamp behavior across the application. Key changes include:

  • Introduction of ClockConfig and ClockProperties for centralized clock management.
  • Modifications to PingMapper and related tests to use injected Clock instead of Instant.now().
  • Updates to the README and application.yml to document and enable timezone configuration.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/test/java/dev/luismachadoreis/flighttracker/server/ping/application/dto/PingMapperTest.java Updated tests to use a fixed Clock for deterministic timestamps.
src/test/java/dev/luismachadoreis/flighttracker/server/flightdata/infrastructure/pubsub/FlightDataSubscriberTest.java Updated test to use fixed Clock for timestamp generation.
src/main/resources/application.yml Added new clock configuration settings (timezone set to UTC by default).
src/main/java/dev/luismachadoreis/flighttracker/server/ping/application/dto/PingMapper.java Modified to use injected Clock for creating timestamps.
src/main/java/dev/luismachadoreis/flighttracker/server/common/infrastructure/ClockProperties.java Added a record for clock properties with default timezone assignment.
src/main/java/dev/luismachadoreis/flighttracker/server/common/infrastructure/ClockConfig.java Provides a Clock bean based on the configured timezone.
README.md Updated documentation to include clock configuration and simplified project structure.

luismr and others added 2 commits April 30, 2025 22:05
…infrastructure/ClockProperties.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@luismr luismr merged commit 4faaee4 into main May 1, 2025
1 check passed
@luismr luismr deleted the hotfix/tests-rely-instant-with-clock branch May 1, 2025 01:16
@DIEGOHORVATTI
Copy link

KKKK LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants