Skip to content

Update workflow to include unit tests #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
pawelrutkaq merged 6 commits into from
Jan 27, 2026
Merged

Update workflow to include unit tests #45

pawelrutkaq merged 6 commits into from
Jan 27, 2026

Conversation

@WilliamRoebuck
Copy link
Contributor

@WilliamRoebuck WilliamRoebuck commented Jan 21, 2026 •
edited
Loading

Moved unit tests from separate directory to src

@github-actions
Copy link

github-actions bot commented Jan 21, 2026 •
edited
Loading

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]
2026/01/26 14:57:28 Downloading https://releases.bazel.build/8.4.2/release/bazel-8.4.2-linux-x86_64...
Extracting Bazel installation...
Starting local Bazel server (8.4.2) and connecting to it...
INFO: Invocation ID: 08860c5f-df41-4982-b95b-b335f7bc886d
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
DEBUG: Rule 'rust_qnx8_toolchain+' indicated that a canonical reproducible form can be obtained by modifying arguments integrity = "sha256-eQOopREOYCL5vtTb6c1cwZrql4GVrJ1FqgxarQRe1xs="
DEBUG: Repository rust_qnx8_toolchain+ instantiated at:
  <builtin>: in <toplevel>
Repository rule http_archive defined at:
  /home/runner/.bazel/external/bazel_tools/tools/build_defs/repo/http.bzl:431:31: in <toplevel>
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 2 packages loaded
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)

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

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

Analyzing: target //:license-check (137 packages loaded, 2424 targets configured)

Analyzing: target //:license-check (153 packages loaded, 4964 targets configured)

Analyzing: target //:license-check (153 packages loaded, 4964 targets configured)

Analyzing: target //:license-check (154 packages loaded, 4964 targets configured)

Analyzing: target //:license-check (158 packages loaded, 9065 targets configured)

Analyzing: target //:license-check (161 packages loaded, 13571 targets configured)

Analyzing: target //:license-check (161 packages loaded, 13571 targets configured)

INFO: Analyzed target //:license-check (162 packages loaded, 13697 targets configured).
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.324s, Critical Path: 0.34s
INFO: 13 processes: 4 disk cache hit, 9 internal.
INFO: Build completed successfully, 13 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>]

@github-actions
Copy link

github-actions bot commented Jan 21, 2026

The created documentation from the pull request is available at: docu-html

@pawelrutkaq
Copy link
Contributor

pawelrutkaq commented Jan 22, 2026

Why we need to split this in a job ? I am not sure if this will not make bazel cold again and increase times.

@WilliamRoebuck
Copy link
Contributor Author

WilliamRoebuck commented Jan 22, 2026

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

pawelrutkaq
pawelrutkaq reviewed Jan 26, 2026
View reviewed changes
.github/workflows/tests.yml Outdated
pull-requests: read
with:
bazel-target: 'test //src/...'
bazel-target: 'test //src/... //tests/ut/...'
Copy link
Contributor

@pawelrutkaq pawelrutkaq Jan 26, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@pawelrutkaq pawelrutkaq Jan 26, 2026

Choose a reason for hiding this comment

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

@PiotrKorkus ?

Copy link

@PiotrKorkus PiotrKorkus Jan 26, 2026

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@WilliamRoebuck WilliamRoebuck Jan 26, 2026

Choose a reason for hiding this comment

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

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

Copy link

@PiotrKorkus PiotrKorkus Jan 26, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@pawelrutkaq pawelrutkaq Jan 26, 2026

Choose a reason for hiding this comment

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

Ye also temaplate src folder is [probably goign to change into score

Copy link

@PiotrKorkus PiotrKorkus Jan 26, 2026

Choose a reason for hiding this comment

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

eclipse-score/module_template#42

@MaciejKaszynski MaciejKaszynski mentioned this pull request Jan 26, 2026
Merged
pawelrutkaq
pawelrutkaq reviewed Jan 26, 2026
View reviewed changes
Copy link
Contributor

@pawelrutkaq pawelrutkaq left a comment

Choose a reason for hiding this comment

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

Besides rebase look good now

src/launch_manager_daemon/health_monitor_lib/src/score/lcm/saf/daemon/PhmDaemon.cpp
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))
Copy link
Contributor

@pawelrutkaq pawelrutkaq Jan 26, 2026

Choose a reason for hiding this comment

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

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

@WilliamRoebuck WilliamRoebuck force-pushed the feature/add-test-actions branch from 88525cc to 3d69487 Compare January 26, 2026 14:56
@pawelrutkaq pawelrutkaq merged commit 046ea28 into eclipse-score:main Jan 27, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PiotrKorkus PiotrKorkus PiotrKorkus left review comments

@pawelrutkaq pawelrutkaq pawelrutkaq approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@WilliamRoebuck @pawelrutkaq @PiotrKorkus