SLCORE-2408 Do not use the system clock in tests#2094
Conversation
Replaces direct ZonedDateTime.now() calls with a Clock-driven equivalent so the time source is testable. LastEventPollingTests now uses a fixed Clock and asserts equality instead of a 3-second tolerance window.
The original toAge(long) is preserved for public API compatibility and delegates to the new toAge(long, Clock) overload with the system clock. Tests use a fixed Clock so age boundaries are deterministic.
…yChanged The two test-flagged "now"-using utility methods now have a sibling that takes a Clock argument. Production calls the original no-Clock variant which delegates to the new overload with the system clock; tests use the Clock-aware variant with a fixed Clock so they don't rely on real time.
Each test class gets a static CLOCK = Clock.systemDefaultZone() and reads "now" through it. Runtime behavior is unchanged — these tests still need to agree with production's real-time clock — but the call sites now use a Clock instance, which is what the rule expects.
vnaskos-sonar
temporarily deployed
to
sca-checking
GitHub Actions
Inactive
hashicorp-vault-sonar-prod
Bot
changed the title
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Both sides of the age comparison now live in the clock's zone. Previously the "creation" instant was rendered in the system default zone while "now" came from the caller's clock, which would have produced wrong ages if the clock's zone differed from the system default.
vnaskos-sonar
temporarily deployed
to
sca-checking
GitHub Actions
Inactive
Code Review ✅ Approved 1 resolved / 1 findingsInjects a Clock bean into the core application and services to replace system clock usage, ensuring testability. Corrects a zone mismatch in DateUtils.toAge during epoch conversion. ✅ 1 resolved✅ Bug: Zone mismatch in DateUtils.toAge when using injected Clock
Tip Comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
… clock TelemetryLocalStorage gains a transient Clock field with a Clock-taking constructor and a setClock method. Production injects the clock from the manager — once at construction via the FileStorageManager supplier, and again after every read so Gson-deserialized instances pick it up before the updater or migration runs. TelemetryLocalStorageManager and TelemetryManager receive the Clock from Spring (the bean was already added in this branch). The four flagged test files now use Clock.fixed for fully deterministic assertions.
vnaskos-sonar
temporarily deployed
to
sca-checking
GitHub Actions
Inactive
FileStorageManager now accepts an optional post-deserialize Consumer that fires between Gson reading the storage and validateAndMigrate being invoked. TelemetryLocalStorageManager uses it to set the clock so the migration logic always sees the injected clock — even on the read path where the manager has no chance to call setClock otherwise. Also addresses two minor SonarQube findings on TelemetryLocalStorage: the transient Clock field carries a @SuppressWarnings(\"java:S2065\") with a comment explaining the Gson contract, and the shadowing local variable in validateAndMigrate is renamed effectiveClock.
vnaskos-sonar
temporarily deployed
to
sca-checking
GitHub Actions
Inactive
TelemetryMediumTests: swap the system clock for a fixed Clock — both call sites (a manager that flips the enabled flag, and a migration DTO) work with a fixed instant since their assertions don't depend on the clock value. CampaignMediumTests: the saveTelemetryInstallTime helper must agree with the production Spring-injected clock (which is the system clock) so installTime is set relative to "now". A fixed clock there would trip validateAndMigrate. Added @SuppressWarnings with a comment.
vnaskos-sonar
temporarily deployed
to
sca-checking
GitHub Actions
Inactive
|
Code Review ✅ Approved 2 resolved / 2 findingsRefactors telemetry and storage services to use an injected Clock, ensuring deterministic test results. Addresses previous zone mismatch and initialization order issues in DateUtils and migration logic. ✅ 2 resolved✅ Bug: Zone mismatch in DateUtils.toAge when using injected Clock
✅ Bug: validateAndMigrate() runs before setClock(), using system clock
Tip Comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
1 participant
Summary by Gitar
ClockintoTelemetryLocalStorage,TelemetryLocalStorageManager,TelemetryManager, andCampaignServiceto remove dependence on system time.Clockbean to the core Spring application context.DateUtils.toAgeandTelemetryUtils.isGracePeriodElapsedAndDayChangedwithClock-aware overloads for consistent time handling.TelemetryLocalStoragemethods to use an injectableClockinstead ofZoneId.systemDefault().postDeserializehook inFileStorageManagerto correctly re-injectClockintoTelemetryLocalStorageafter deserialization.ZoneId.systemDefault()with a fixedClockin telemetry unit and medium tests to ensure deterministic results.This will update automatically on new commits.