Skip to content

Add thermal service unit test - #950

Merged
jerrysxie merged 3 commits into
OpenDevicePartnership:mainfrom
jerrysxie:add-thermal-service-unit-test
Aug 24, 2026
Merged

Add thermal service unit test#950
jerrysxie merged 3 commits into
OpenDevicePartnership:mainfrom
jerrysxie:add-thermal-service-unit-test

Conversation

@jerrysxie

Copy link
Copy Markdown
Contributor

Add new unit test to thermal service with the assistance of AI, discover 2 quirks in the thermal service behavior that needs some clarifications

File Before (line) After (line) Δ
sensor.rs 0/138 (0%) 133/138 (96.4%) +96.4
fan.rs 0/225 (0%) 215/225 (95.6%) +95.6
utils.rs 0/51 (0%) 51/51 (100%) +100.0
lib.rs 0/16 (0%) 0/16 (0%) 0.0
TOTAL 0/430 (0%) 399/430 (92.79%) +92.79

Exercise fan state transitions, manual overrides, sampling, and failure
events through the service runner.

Cover sensor retries, thresholds, hysteresis, sampling control, and
buffer behavior.

Assisted-by: GitHub Copilot:GPT 5.3 Codex
Extend the fan and sensor service test suites to close coverage
gaps identified in review.

Sensor: assert immediate reads ignore offset, exercise the
threshold getter and sample-period setter, cover the fast-sampling
branch, verify event fan-out to multiple senders, and check
prochot/critical hysteresis clearing.

Fan: cover the cool-down state transitions through hysteresis, the
RPM sampling and update period setters, the rpm_immediate error
path, and event fan-out to multiple senders. Add a scripted
RPM-read failure hook to the fan test fake to enable the error path.

Assisted-by: GitHub Copilot:claude-opus-4.8
@jerrysxie
jerrysxie requested a review from kurtjd August 21, 2026 22:03
@jerrysxie jerrysxie self-assigned this Aug 21, 2026

@jerrysxie jerrysxie left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@kurtjd Some of these tests are a bit low value because the AI agent was focused on grcov metric. Feel free to suggest removal of any test that you deem as unnecessary.

Comment thread thermal-service/tests/sensor.rs Outdated
Comment thread thermal-service/tests/fan.rs
@jerrysxie
jerrysxie marked this pull request as ready for review August 21, 2026 22:08
@jerrysxie
jerrysxie requested a review from a team as a code owner August 21, 2026 22:08
@jerrysxie
jerrysxie requested review from RobertZ2011, tullom and williampMSFT and a lite review from Copilot and removed request for Copilot August 21, 2026 22:08
Comment thread thermal-service/tests/fan.rs
Comment thread thermal-service/tests/fan.rs
Comment thread thermal-service/tests/sensor.rs
kurtjd
kurtjd previously approved these changes Aug 24, 2026
Apply the configured sensor offset to immediate temperature reads.

This keeps immediate values consistent with periodically sampled values.

Update the regression test to cover the corrected result.

Assisted-by: GitHub Copilot:claude-opus-4.6
Copilot AI lite review requested due to automatic review settings August 24, 2026 17:13
@jerrysxie
jerrysxie requested a review from RobertZ2011 August 24, 2026 17:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 expands thermal-service test coverage by adding new integration tests for the sensor and fan services, plus unit tests for SampleBuf, and it aligns temperature_immediate() behavior with the runner’s offset handling.

Changes:

  • Add comprehensive #[tokio::test] integration tests for thermal_service::sensor and thermal_service::fan behaviors (thresholds, hysteresis, enable/disable flows, retries, and event broadcasting).
  • Fix thermal-service sensor API behavior so temperature_immediate() applies the configured offset.
  • Add SampleBuf unit tests and introduce thermal-service dev-dependencies needed to run the new async tests on std.

Reviewed changes

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

Show a summary per file
File Description
thermal-service/tests/sensor.rs New integration tests covering sensor retries, offset, sampling enable/disable, thresholds/hysteresis, and event fan-out.
thermal-service/tests/fan.rs New integration tests covering fan auto-control state transitions, manual overrides, sampling averages, failure events, and event fan-out.
thermal-service/src/utils.rs Adds unit tests for SampleBuf behavior (empty/default, average, eviction, accumulator width).
thermal-service/src/sensor.rs Makes temperature_immediate() apply the configured offset, matching sampled temperature behavior.
thermal-service/Cargo.toml Adds std/dev deps (tokio, embassy-* std features, critical-section/std) for the new tests.
Cargo.lock Records the new dev-dependency additions for thermal-service.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread thermal-service/tests/fan.rs
@jerrysxie
jerrysxie enabled auto-merge (squash) August 24, 2026 19:02
@jerrysxie
jerrysxie merged commit b5961ed into OpenDevicePartnership:main Aug 24, 2026
16 checks passed
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.

5 participants