Skip to content

Conversation

@arkjedrz
Copy link
Contributor

@arkjedrz arkjedrz commented Aug 1, 2025

  • Remove redundant files.
  • Reduce number of test files.
  • Move common.rs to common/mod.rs as recommended by Rust Book.
  • Uniform derive usage in kvs_api.rs.
  • Bazel target for unit tests.

@arkjedrz arkjedrz force-pushed the arkjedrz_reorg-tests branch from 4dad25d to 3385d00 Compare August 1, 2025 12:01
@arkjedrz arkjedrz self-assigned this Aug 1, 2025
@github-actions
Copy link

github-actions bot commented Aug 1, 2025

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 and connecting to it...
INFO: Invocation ID: 08201df9-9f6f-414e-8119-186bfb7fdea6
Computing main repo mapping: 
Computing main repo mapping: 
DEBUG: Rule 'rules_boost~' indicated that a canonical reproducible form can be obtained by modifying arguments integrity = "sha256-aSViuHoOdF32Ue/Cx02/mOzyEebHMFuifjZs4GNWjsE="
DEBUG: Repository rules_boost~ instantiated at:
  <builtin>: in <toplevel>
Repository rule http_archive defined at:
  /home/runner/.bazel/external/bazel_tools/tools/build_defs/repo/http.bzl:387:31: in <toplevel>
Computing main repo mapping: 
WARNING: For repository 'googletest', the root module requires module version googletest@1.14.0, but got googletest@1.15.0 in the resolved dependency graph.
WARNING: For repository 'aspect_rules_lint', the root module requires module version aspect_rules_lint@1.0.3, but got aspect_rules_lint@1.4.2 in the resolved dependency graph.
Computing main repo mapping: 
Loading: 
Loading: 1 packages loaded
Analyzing: target //:license-check (2 packages loaded, 0 targets configured)
Analyzing: target //:license-check (2 packages loaded, 0 targets configured)

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

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

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

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

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

Analyzing: target //:license-check (168 packages loaded, 4816 targets configured)

Analyzing: target //:license-check (170 packages loaded, 5060 targets configured)

Analyzing: target //:license-check (170 packages loaded, 5060 targets configured)

INFO: Analyzed target //:license-check (171 packages loaded, 6979 targets configured).
[9 / 13] [Prepa] JavaToolchainCompileClasses external/rules_java~/toolchains/platformclasspath_classes
ERROR: /home/runner/work/inc_mw_per/inc_mw_per/BUILD:36:21: Generating Dash formatted dependency file ... failed: missing input file '//:cargo_lock'
ERROR: /home/runner/work/inc_mw_per/inc_mw_per/BUILD:36:21: Generating Dash formatted dependency file ... failed: 1 input file(s) do not exist
Target //:license.check.license_check failed to build
Use --verbose_failures to see the command lines of failed build steps.
ERROR: /home/runner/work/inc_mw_per/inc_mw_per/BUILD:36:21 Generating Dash formatted dependency file ... failed: 1 input file(s) do not exist
INFO: Elapsed time: 184.427s, Critical Path: 0.31s
INFO: 10 processes: 1 disk cache hit, 9 internal.
ERROR: Build did NOT complete successfully
ERROR: Build failed. Not running target

@github-actions
Copy link

github-actions bot commented Aug 1, 2025

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

Copy link
Contributor

@PiotrKorkus PiotrKorkus left a comment

Choose a reason for hiding this comment

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

In general the concept of keeping unit tests together with implementation is the proffered way according to Rust documentation. There are different opinions on that in SCORE but as long as file doesn't look like 10% implementation, 90% tests I think it is a good move.

@arkjedrz arkjedrz force-pushed the arkjedrz_reorg-tests branch 2 times, most recently from 83d73da to 58f7fdf Compare August 1, 2025 14:44
Copy link
Contributor

@PiotrKorkus PiotrKorkus left a comment

Choose a reason for hiding this comment

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

Looks good to me, let's wait for final decision from Vinod

- Remove redundant files.
- Reduce number of test files.
- Move `common.rs` to `common/mod.rs` as recommended by Rust Book.
- Uniform `derive` usage in `kvs_api.rs`.
- Bazel target for unit tests.
@arkjedrz arkjedrz force-pushed the arkjedrz_reorg-tests branch from 58f7fdf to e9b7ce0 Compare August 5, 2025 06:34
@vinodreddy-g vinodreddy-g merged commit b1ea6e3 into eclipse-score:main Aug 6, 2025
11 checks passed
@arkjedrz arkjedrz deleted the arkjedrz_reorg-tests branch September 8, 2025 07:13
atarekra pushed a commit to Valeo-S-CORE-Organization/persistency that referenced this pull request Nov 9, 2025
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.

3 participants