feat: add TimeDaemon CIT with pip hub infrastructure - #122
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run //:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
db540ed to
d765fee
Compare
There was a problem hiding this comment.
Sorry, I don't understand this: What is the benefit of using the combination of the pytest/testing_utils plus C++ code over using plain gtest/gmock? At the moment it looks to me making things more complicated and harder to understand.
Imho, for all the component internal testing, I would use gtest. That doesn't mean that everything tested with gtest is automatically a unit test (in the strict sense), it can be also some sort of integration tests - i.e. test which integrate multiple classes and test them together.
Then on the level of testing the whole component as a binary - TimeDaemon in this case - I'd use some pytests framework or ITF to stimulate and test the binary from "outside". And next step would be testing of multiple binaries "playing together".
There was a problem hiding this comment.
Thanks, fair point. You're right that ptp_divider and the single-validator scenarios were duplicating gtest coverage with unnecessary indirection — I've removed them in the latest push.
The CIT layer now only keeps scenarios that wire together two or more production objects: verification.pipeline (PTP stub → verification machine), ipc.shm_roundtrip (publisher/receiver over shared memory), and daemon.lifecycle (full SvtHandler init/shutdown). Single-class behaviour stays with gtest under src/. Added a short README documenting the boundary.
The pytest + test_scenarios stack is the same SCORE-standard framework used for the upcoming module-integration and system-test layers (multi-binary, QEMU), so keeping it here gives one runner surface scaling up to those levels. Let me know if that still feels off.
f74e6e9 to
321294a
Compare
| psutil | ||
| pytest-metadata | ||
| pytest-env | ||
| testing-utils @ git+https://github.com/eclipse-score/testing_tools.git@a2f9cded3deb636f5dc800bf7a47131487119721 |
There was a problem hiding this comment.
Please add version tag as comment
| testing-utils @ git+https://github.com/eclipse-score/testing_tools.git@a2f9cded3deb636f5dc800bf7a47131487119721 | |
| testing-utils @ git+https://github.com/eclipse-score/testing_tools.git@a2f9cded3deb636f5dc800bf7a47131487119721 # v0.3.0 |
| # via testing-utils | ||
| # WARNING: pip install will require the following package to be hashed. | ||
| # Consider using a hashable URL like https://github.com/jazzband/pip-tools/archive/SOMECOMMIT.zip | ||
| testing-utils @ git+https://github.com/eclipse-score/testing_tools.git@v0.3.0 |
There was a problem hiding this comment.
Oops - why is the hash here replaced by the version tag? Thought this is generated ...
| void run(const std::string& /*input*/) const final | ||
| { | ||
| // Phase 1: construct — calls all four subsystem factory functions | ||
| auto handler = score::td::CreateSvtTimebase(); | ||
|
|
||
| // Phase 2: initialize — wires MessageBroker pub-sub topology | ||
| handler->Initialize(); | ||
|
|
||
| // Phase 3: stop from kIdle — verifies graceful shutdown without starting async workers | ||
| handler->Stop(); | ||
|
|
||
| TRACING_INFO(kTargetName, | ||
| std::pair{std::string{"lifecycle_initialize_ok"}, std::string{"true"}}, | ||
| std::pair{std::string{"lifecycle_complete"}, std::string{"true"}}); | ||
| } |
There was a problem hiding this comment.
This is already tested in score/time_daemon/src/application/svt/svt_handler_integration_test.cpp
| void run(const std::string& /*input*/) const final | ||
| { | ||
| auto publisher = score::td::CreateSvtPublisher("cit_ipc_pub"); | ||
| auto receiver = score::td::CreateSvtReceiver(); | ||
|
|
||
| if (!publisher->Init()) | ||
| { | ||
| throw std::runtime_error{"SvtPublisher::Init() failed"}; | ||
| } | ||
| if (!receiver->Init()) | ||
| { | ||
| throw std::runtime_error{"SvtReceiver::Init() failed"}; | ||
| } | ||
|
|
||
| score::td::PtpTimeInfo info{}; | ||
| info.ptp_assumed_time = std::chrono::nanoseconds{1'000'000'000LL}; | ||
| info.local_time = score::td::PtpTimeInfo::ReferenceClock::time_point{std::chrono::nanoseconds{500'000'000LL}}; | ||
| info.rate_deviation = 0.0; | ||
| info.status = {true, false, false, false, true}; | ||
| info.sync_fup_data.sequence_id = 42U; | ||
| info.sync_fup_data.precise_origin_timestamp = 1'000'000'000ULL; | ||
|
|
||
| publisher->OnMessage(info); | ||
|
|
||
| const auto result = receiver->Receive(); | ||
| const bool read_ok = result.has_value(); | ||
|
|
||
| bool ptp_time_ok = false; | ||
| bool status_ok = false; | ||
| bool seq_ok = false; | ||
|
|
||
| if (read_ok) | ||
| { | ||
| const auto& snap = result.value(); | ||
| ptp_time_ok = snap.ptp_assumed_time == static_cast<uint64_t>(1'000'000'000LL); | ||
| status_ok = snap.status.is_synchronized && snap.status.is_correct; | ||
| seq_ok = snap.sync_fup_data.sequence_id == 42U; | ||
| } | ||
|
|
||
| TRACING_INFO(kTargetName, | ||
| std::pair{std::string{"read_succeeded"}, std::string{read_ok ? "true" : "false"}}, | ||
| std::pair{std::string{"ptp_time_preserved"}, std::string{ptp_time_ok ? "true" : "false"}}, | ||
| std::pair{std::string{"status_preserved"}, std::string{status_ok ? "true" : "false"}}, | ||
| std::pair{std::string{"seq_id_preserved"}, std::string{seq_ok ? "true" : "false"}}); | ||
| } |
There was a problem hiding this comment.
This is also already tested: score/time_daemon/src/ipc/svt/common_factory_test.cpp
| { | ||
| auto ptp_machine = score::td::CreateGPTPStubMachine("cit_verif_ptp"); | ||
| auto verifier = score::td::CreateSvtVerificationMachine("cit_verif"); | ||
|
|
||
| std::promise<score::td::PtpTimeInfo> verified_promise; | ||
| auto verified_future = verified_promise.get_future(); | ||
|
|
||
| // Wire PTP machine output into the verification pipeline. | ||
| ptp_machine->SetPublishCallback([&verifier](const score::td::PtpTimeInfo& data) { | ||
| verifier->OnMessage(data); | ||
| }); | ||
|
|
||
| // Capture the first data point that exits the verification pipeline. | ||
| verifier->SetPublishCallback([&verified_promise](const score::td::PtpTimeInfo& data) { | ||
| try | ||
| { | ||
| verified_promise.set_value(data); | ||
| } | ||
| catch (const std::future_error&) | ||
| { | ||
| // Capture only the first verified data point. | ||
| } | ||
| }); | ||
|
|
||
| verifier->Init(); | ||
|
|
||
| if (!ptp_machine->Init()) | ||
| { | ||
| throw std::runtime_error{"GPTPStubMachine::Init() failed"}; | ||
| } | ||
| ptp_machine->Start(); | ||
|
|
||
| if (verified_future.wait_for(std::chrono::seconds(5)) != std::future_status::ready) | ||
| { | ||
| ptp_machine->Stop(); | ||
| throw std::runtime_error{"Timed out waiting for verified PTP data through pipeline"}; | ||
| } |
There was a problem hiding this comment.
This kind of test makes sense, because it really combines multiple sub-components. Following remarks:
- We should use plain gtest for doing this. Makes error checking much easier and expressive, e.g. instead of having:
if (!ptp_machine->Init()) {
throw std::runtime_error{"GPTPStubMachine::Init() failed"};
}
we'd have:
ASSERT_TRUE(ptp_machine->Init());
and no additional checks on Python side.
- A similar test like this could be achieved by instantiating the application svt_handler. Valery already added test code there (score/time_daemon/src/application/svt/svt_handler_integration_test.cpp) but that is commented out at the moment.
I really would use Python/ITF based tests only for testing a single - in this case the TimeDaemon - or multiple binaries working together.
This PR adds TimeDaemon component integration tests with pip hub infrastructure (part 1 of 3):
Part of #56
test QNX