Skip to content

test fixed#1024

Merged
GonzoSpire merged 1 commit intomasterfrom
TAM-2037
Mar 24, 2026
Merged

test fixed#1024
GonzoSpire merged 1 commit intomasterfrom
TAM-2037

Conversation

@GonzoSpire
Copy link
Copy Markdown
Collaborator

No description provided.

@github-actions
Copy link
Copy Markdown

PR Review: TAM-2037 — "test fixed"

Files changed: 2 test files, 3 lines modified
Verdict: LGTM ✓ — correct fix, no issues found


Summary

This PR fixes three test assertions/setups that were using ReceivingTime where Time should be used. The changes are semantically correct given the BaseValue model's design.


Analysis

Why the old code was wrong:

BaseValue has two distinct timestamp properties:

  • ReceivingTime { get; init; } = DateTime.UtcNow — server-side ingestion timestamp, init-only, not used in the LastUpdate computation chain.
  • Time { get; set; } — the sensor reading timestamp, used via LastUpdateTime = LastReceivingTime ?? Time.

BaseSensorModel.LastUpdate resolves as:

LastUpdate → Storage.LastValue?.LastUpdateTime ?? DateTime.MinValue
LastUpdateTime → LastReceivingTime ?? Time

ReceivingTime plays no role here, so tests using it to control timeout behavior or verify LastUpdate were testing against the wrong property.


Change-by-change review

1. ModelsTester.cs:161 — Assertion fix

- Assert.Equal(expectedSensorValue.ReceivingTime, actual.LastUpdate);
+ Assert.Equal(expectedSensorValue.Time, actual.LastUpdate);

Correct. actual.LastUpdate ultimately falls back to Time (when LastReceivingTime is null), not ReceivingTime. The assertion now checks the property that is actually referenced.

2. BaseSensorModelValidatorTests.cs:141-142 — TTL timeout test setup

- { ReceivingTime = new DateTime(DateTime.UtcNow.Ticks - ticks) };
+ { Time = new DateTime(DateTime.UtcNow.Ticks - ticks) };

Correct. CheckTimeout() checks LastUpdate, which flows through LastUpdateTime = LastReceivingTime ?? Time. Setting an old Time on the value correctly simulates a stale sensor reading for the timeout test.

3. BaseSensorModelValidatorTests.cs:187-188 — Combined status/timeout test setup

- ReceivingTime = new DateTime(DateTime.UtcNow.Ticks - TestTicks),
+ Time = new DateTime(DateTime.UtcNow.Ticks - TestTicks),

Same rationale as #2. Correct fix.


Issues found

None. No bugs, security concerns, or logic errors introduced. The changes are minimal and targeted.


Minor note (non-blocking)

The PR title "test fixed" is quite terse. A slightly more descriptive title like "Fix tests to use Time instead of ReceivingTime for LastUpdate assertions" would make the git history more navigable, but this is a stylistic preference.

@GonzoSpire GonzoSpire merged commit 8312e8d into master Mar 24, 2026
1 check passed
@GonzoSpire GonzoSpire deleted the TAM-2037 branch March 24, 2026 11:04
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.

1 participant