Skip to content

Improve time daemon coverage - #205

Open
MaciejSalwa543 wants to merge 5 commits into
eclipse-score:mainfrom
qorix-group:improve-time-daemon-coverage
Open

Improve time daemon coverage#205
MaciejSalwa543 wants to merge 5 commits into
eclipse-score:mainfrom
qorix-group:improve-time-daemon-coverage

Conversation

@MaciejSalwa543

@MaciejSalwa543 MaciejSalwa543 commented Sep 2, 2026

Copy link
Copy Markdown

Add test cases closing branch/line coverage gaps in msg_broker, ptp_machine, verification_machine, TimeJumpsValidator, and TimeoutValidator, plus PrintTo coverage for ptp_time_info and svt_time_info.

Justify the remaining defensive/unreachable branches in tools/coverage/coverage_justifications.yaml.

Verified locally: bazel test //score/time_daemon/... (22/22 pass) and
bazel coverage --config=llvm_cov //score/time_daemon/... — line coverage 96.85%, branch coverage
~92%.

No production code was modified — this PR only adds tests and coverage justifications.

Related ticket

closes #45

@MaciejSalwa543
time_daemon: Increase unit test coverage toward 90% line and branch
ef6e7a0 
Add test cases closing branch/line coverage gaps in msg_broker, ptp_machine,
verification_machine, TimeJumpsValidator, and TimeoutValidator, plus PrintTo
coverage for ptp_time_info and svt_time_info. Justify the remaining
defensive/unreachable branches in tools/coverage/coverage_justifications.yaml.

Refs eclipse-score#45

Signed-off-by: Maciej Salwa <maciej.salwa.ext@qorix.ai>
@MaciejSalwa543
time_daemon: Address review feedback on coverage tests
d4359f6 
Convert per-field equality mutation tests to TEST_P (ptp_time_info_test.cpp)
so each field is its own named test case. Rename msg_broker tests to match
what they actually verify, rewrite ExpiredBrokerDoesNotProcessCallback to
use a live consumer so non-delivery is observable, and rename the two
ptp_machine tests to describe observable behavior rather than log output.
Move the two VerificationMachine death tests into a dedicated
VerificationMachineDeathTest suite per GTest convention.

Refs eclipse-score#45

Signed-off-by: Maciej Salwa <maciej.salwa.ext@qorix.ai>
@MaciejSalwa543
time_daemon: Use MOCK_METHOD for MockConsumer::OnMessage
742d93b 
Switch MockConsumer::OnMessage to a real gmock MOCK_METHOD with a default
action that still populates received_data, so ExpiredSubscriberDoesNotReceiveData
can assert Times(0) instead of only relying on EXPECT_NO_THROW.

Refs eclipse-score#45

Signed-off-by: Maciej Salwa <maciej.salwa.ext@qorix.ai>
@github-project-automation github-project-automation Bot moved this to In Progress in TIM - Time FT Sep 2, 2026
@MaciejSalwa543
MaciejSalwa543 deployed to workflow-approval September 2, 2026 11:03 — with GitHub Actions Active
@MaciejSalwa543
MaciejSalwa543 deployed to workflow-approval September 2, 2026 11:03 — with GitHub Actions Active
@MaciejSalwa543
MaciejSalwa543 marked this pull request as draft September 2, 2026 11:03
@github-actions

github-actions Bot commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //:license-check

Status: ⚠️ Needs Review

Click to expand output 
[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.6.0) and connecting to it...
INFO: Invocation ID: 3554f405-9f21-4015-a215-605e779683f6
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
WARNING: For repository 'platforms', the root module requires module version platforms@1.0.0, but got platforms@1.1.0 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 3 packages loaded
Analyzing: target //:license-check (4 packages loaded, 0 targets configured)
Analyzing: target //:license-check (4 packages loaded, 0 targets configured)

Analyzing: target //:license-check (84 packages loaded, 10 targets configured)

Analyzing: target //:license-check (151 packages loaded, 3145 targets configured)

Analyzing: target //:license-check (160 packages loaded, 7064 targets configured)

Analyzing: target //:license-check (165 packages loaded, 9329 targets configured)

Analyzing: target //:license-check (167 packages loaded, 9368 targets configured)

Analyzing: target //:license-check (172 packages loaded, 9417 targets configured)

Analyzing: target //:license-check (175 packages loaded, 11305 targets configured)

Analyzing: target //:license-check (176 packages loaded, 11429 targets configured)

Analyzing: target //:license-check (176 packages loaded, 11429 targets configured)

INFO: Analyzed target //:license-check (177 packages loaded, 11555 targets configured).
INFO: From Generating Dash formatted dependency file ...:
INFO: Successfully converted 2 packages from Cargo.lock to bazel-out/k8-fastbuild/bin/formatted.txt
[14 / 16] [Prepa] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar
[15 / 16] Building license.check.license_check.jar (); 0s disk-cache, multiplex-worker
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 26.665s, Critical Path: 2.20s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

@pawelrutkaq
pawelrutkaq marked this pull request as ready for review September 2, 2026 11:04
pawelrutkaq
pawelrutkaq reviewed Sep 2, 2026
View reviewed changes

@pawelrutkaq pawelrutkaq left a comment •
edited
Loading

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.

LGTM - Reviewed by me internally I will merge once some cft collegues approve/comments will be resolved

@MaciejSalwa543
time_daemon: Fix clang-format and yamlfmt violations
9a7d7c2 
Run //:format.fix to reformat the TEST_P conversion in ptp_time_info_test.cpp,
the ASSERT_DEATH call in verification_machine_test.cpp, and collapse the new
coverage_justifications.yaml reason blocks to a single line each, matching
CI's format.check gate.

Refs eclipse-score#45

Signed-off-by: Maciej Salwa <maciej.salwa.ext@qorix.ai>
@MaciejSalwa543
MaciejSalwa543 deployed to workflow-approval September 2, 2026 11:45 — with GitHub Actions Active
@MaciejSalwa543
MaciejSalwa543 deployed to workflow-approval September 2, 2026 11:45 — with GitHub Actions Active
lavrovvalera
lavrovvalera reviewed Sep 2, 2026
View reviewed changes
Comment thread score/time_daemon/src/common/data_types/ptp_time_info_test.cpp Outdated
@MaciejSalwa543
time_daemon: Justify GTest-only PrintTo hooks instead of testing them
cc458f4 
Drop the PrintTo tests in ptp_time_info_test.cpp and svt_time_info_test.cpp;
those PrintTo(..., std::ostream*) overloads are only ever called by GTest's
own failure-printing machinery, not application code, so a test that just
calls them directly was coverage-for-its-own-sake. Justify them in
coverage_justifications.yaml instead (category: other, no existing category
fit better).

Refs eclipse-score#45

Signed-off-by: Maciej Salwa <maciej.salwa.ext@qorix.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pawelrutkaq pawelrutkaq pawelrutkaq left review comments
+1 more reviewer
@lavrovvalera lavrovvalera lavrovvalera left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

TIM - Time FT
Status: In Progress

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improvement: Increase unit test coverage to 90% line + branch: score/TimeDaemon/

3 participants

@MaciejSalwa543 @lavrovvalera @pawelrutkaq