Update workflow to include unit tests#45
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 |
|
Why we need to split this in a job ? I am not sure if this will not make bazel cold again and increase times. |
Good point, I've combined the tests now. I think it would be nice to have separate jobs in future to make it clearer what failed but it would require a more complex workflow to keep the cache between tests |
.github/workflows/tests.yml
Outdated
| pull-requests: read | ||
| with: | ||
| bazel-target: 'test //src/...' | ||
| bazel-target: 'test //src/... //tests/ut/...' |
There was a problem hiding this comment.
This directory is not for UT afaik, but for cit tests. So we shall move UT from there is put there and not run it here.
There was a problem hiding this comment.
correct, UTs are located among the implementation. Please take a look e.g.: https://github.com/eclipse-score/baselibs/blob/main/score/mw/log/BUILD
There was a problem hiding this comment.
I don't think we should follow the baselibs layout for this - according to the s-core module template, both unit and integration tests should be in the tests/ directory:
https://github.com/eclipse-score/module_template/blob/092be5c428a9d1c96ec7a0e02522107a591794bc/README.md?plain=1#L21
There was a problem hiding this comment.
Then the template shall be updated - I will handle that.
All other features are storing UTs side to side with sources. That's also a good practice for rust where unit tests are recommended to be kept in the same file as implementation.
There was a problem hiding this comment.
Ye also temaplate src folder is [probably goign to change into score
pawelrutkaq
left a comment
There was a problem hiding this comment.
Besides rebase look good now
| PhmDaemon::PhmDaemon(score::lcm::saf::timers::OsClockInterface& f_osClock, logging::PhmLogger& f_logger_r, | ||
| std::unique_ptr<watchdog::IWatchdogIf> f_watchdog, std::unique_ptr<score::lcm::IProcessStateReceiver> f_process_state_receiver) : | ||
| osClock{f_osClock}, cycleTimer{&osClock}, logger_r{f_logger_r}, swClusterHandlers{}, watchdog(std::move(f_watchdog)), processStateReader{std::move(f_process_state_receiver)} | ||
| osClock{f_osClock}, cycleTimer{&osClock}, logger_r{f_logger_r}, swClusterHandlers{}, processStateReader{std::move(f_process_state_receiver)}, watchdog(std::move(f_watchdog)) |
There was a problem hiding this comment.
I think you did not rebased on main, this change is already in or ? https://github.com/eclipse-score/lifecycle/blob/main/src/launch_manager_daemon/health_monitor_lib/src/score/lcm/saf/daemon/PhmDaemon.cpp#L40
88525cc to
3d69487
Compare
Moved unit tests from separate directory to src