[#476] Introduce test feature in iceoryx-bb for bump_allocator

[xieyuschen]

Notes for Reviewer

See proposal in the issue. By the way, could I know the rules/requirements to be met for a collaborator? Currently, I cannot assign you guys as reviewers:(

Pre-Review Checklist for the PR Author

1. [X] Add sensible notes for the reviewer
2. [X] PR title is short, expressive and meaningful
3. [X] Relevant issues are linked in the References section
4. [X] Every source code file has a copyright header with SPDX-License-Identifier: Apache-2.0 OR MIT
5. [X] Branch follows the naming format (iox2-123-introduce-posix-ipc-example)
6. [X] Commits messages are according to this guideline
   • [X] Commit messages have the issue ID ([#123] Add posix ipc example)
   • [X] Commit author matches Eclipse Contributor Agreement (and ECA is signed)
7. [X] Tests follow the best practice for testing
8. [X] Changelog updated in the unreleased section including API breaking changes
9. [x] Assign PR to reviewer
10. [ ] All checks have passed (except task-list-completed)

Checklist for the PR Reviewer

• [x] Commits are properly organized and messages are according to the guideline
• [x] Unit tests have been written for new behavior
• [x] Public API is documented
• [x] PR title describes the changes

Post-review Checklist for the PR Author

1. [x] All open points are addressed and tracked via issues

References

Closes #476

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 79.22%. Comparing base (5f6a0dc) to head (8ffa846). Report is 6 commits behind head on main.

Additional details and impacted files

[](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx2/pull/478?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx) ```diff @@ Coverage Diff @@ ## main #478 +/- ## ========================================== + Coverage 79.19% 79.22% +0.02% ========================================== Files 200 200 Lines 23716 23716 ========================================== + Hits 18781 18788 +7 + Misses 4935 4928 -7 ``` | [Files with missing lines](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx2/pull/478?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx) | Coverage Δ | | |---|---|---| | [iceoryx2-bb/elementary/src/bump\_allocator.rs](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx2/pull/478?src=pr&el=tree&filepath=iceoryx2-bb%2Felementary%2Fsrc%2Fbump_allocator.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx#diff-aWNlb3J5eDItYmIvZWxlbWVudGFyeS9zcmMvYnVtcF9hbGxvY2F0b3IucnM=) | `86.36% <ø> (ø)` | | ... and [3 files with indirect coverage changes](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx2/pull/478/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx)

[elfenpiff]

@xieyuschen I really like the approach - great idea! I am pinging @elBoberido since he is a bit more familiar with the idioms of Rust.

[xieyuschen]

The unstable pipeline fails for some reasons but it's not caused by the change here. I have cherry-picked this change and run it in my main branch and all pipelines passed. https://github.com/xieyuschen/iceoryx2/commit/8e6a4a0dbbb9c1bacec2b2cf2e5b85139e81a35b

[elBoberido]

@xieyuschen it's caused by an update of the nightly toolchain. A few months ago we alreasy added a workaround to those two tests. Let's wait for a few days to see if another nightly toolchain update fixes the problem. If it persists, we will have a closer look again.

Btw, I still did not manage to check whether this approach works with bazel. I'm also on the ROSCon the next days, so it might take till Friday until I have time

[xieyuschen]

@xieyuschen it's caused by an update of the nightly toolchain. A few months ago we alreasy added a workaround to those two tests. Let's wait for a few days to see if another nightly toolchain update fixes the problem. If it persists, we will have a closer look again.

Btw, I still did not manage to check whether this approach works with bazel. I'm also on the ROSCon the next days, so it might take till Friday until I have time

@elBoberido ack, seems it doesn't block the merging. could you let me know how to test it in bazel? i can help you to verify it:)

[elBoberido]

You basically need to run bazel test //... on your branch after rebasing to main, once #482 is merged. With the feature, it will fail and you need to add crate_features = [ "testing" ], to the rust_test_suite of the failing tests, right below srcs to kerp it consistent with other cases.

[elBoberido]

@xieyuschen it's now merged and you can try your luck. After writing the last comment, I'm convinced it should work without issues.

[xieyuschen]

@xieyuschen it's now merged and you can try your luck. After writing the last comment, I'm convinced it should work without issues.

Resolve this comment and discuss the mac issue in https://github.com/eclipse-iceoryx/iceoryx2/issues/486.

@elBoberido I have checked out to main and run bazel test //... but got this error. Looks like the bindgen-cli has some problems and I'm checking it.

Error log of 'bazel test //... --sandbox_debug '

``` WARNING: Streamed test output requested. All tests will be run locally, without sharding, one at a time ERROR: /Users/yuchen.xie/workspace/pproject/iceoryx2/iceoryx2-pal/posix/BUILD.bazel:39:8: Executing genrule //iceoryx2-pal/posix:iceoryx2-pal-posix-bindgen failed: (Exit 126): sandbox-exec failed: error executing Genrule command (cd /private/var/tmp/_bazel_yuchen.xie/1ba0636c2f2e67e1535f30bfb79eec31/sandbox/darwin-sandbox/27/execroot/_main && \ exec env - \ CARGO_BAZEL_REPIN=true \ PATH=/Users/yuchen.xie/Library/Caches/bazelisk/downloads/sha256/79e4d2401b1c969d2b57babd0c50b0ca61b719689d8b029c919525744f52872c/bin:/Users/yuchen.xie/workspace/go/bin:/Users/yuchen.xie/.cabal/bin:/Users/yuchen.xie/.local/bin:/opt/homebrew/opt/llvm@16/bin:~/.local/bin/:/Users/yuchen.xie/.ghcup/bin:/Users/yuchen.xie/go/bin:/Users/yuchen.xie/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Library/Apple/usr/bin:/usr/local/share/dotnet:~/.dotnet/tools:/usr/local/go/bin:/opt/podman/bin:/Users/yuchen.xie/Library/pnpm:/Users/yuchen.xie/workspace/go/bin:/Users/yuchen.xie/.cabal/bin:/Users/yuchen.xie/.local/bin:/opt/homebrew/opt/llvm@16/bin:~/.local/bin/:/Users/yuchen.xie/.ghcup/bin:/Users/yuchen.xie/go/bin:/Users/yuchen.xie/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/Users/yuchen.xie/.cargo/bin:/Users/yuchen.xie/.orbstack/bin:/Users/yuchen.xie/.orbstack/bin \ RUST_TEST_THREADS=1 \ TMPDIR=/var/folders/28/7sypdf0519xfl4ylhq_90s0m0000gp/T/ \ /usr/bin/sandbox-exec -f /private/var/tmp/_bazel_yuchen.xie/1ba0636c2f2e67e1535f30bfb79eec31/sandbox/darwin-sandbox/27/sandbox.sb /var/tmp/_bazel_yuchen.xie/install/5b64cfdb8a44c51dfff6cba1c4581101/process-wrapper '--timeout=0' '--kill_delay=15' '--stats=/private/var/tmp/_bazel_yuchen.xie/1ba0636c2f2e67e1535f30bfb79eec31/sandbox/darwin-sandbox/27/stats.out' /bin/bash -c 'source external/bazel_tools/tools/genrule/genrule-setup.sh; external/bindgen/bindgen --use-core --blocklist-type max_align_t iceoryx2-pal/posix/src/c/posix.h --output bazel-out/darwin_arm64-fastbuild/bin/iceoryx2-pal/posix/posix_generated.rs') /bin/bash: external/bindgen/bindgen: cannot execute binary file Use --verbose_failures to see the command lines of failed build steps. ```

add on 1: i have changed the bindgen and now there is another problem about cbindgen as /bin/bash: external/cbindgen/file/downloaded: cannot execute binary file

     name = "bindgen",
     repo_rule = http_archive,
-    sha256 = "b7e2321ee8c617f14ccc5b9f39b3a804db173ee217e924ad93ed16af6bc62b1d",
-    strip_prefix = "bindgen-cli-x86_64-unknown-linux-gnu",
-    urls = ["https://github.com/rust-lang/rust-bindgen/releases/download/v0.69.5/bindgen-cli-x86_64-unknown-linux-gnu.tar.xz"],
+    strip_prefix = "bindgen-cli-aarch64-apple-darwin",
+    urls = ["https://github.com/rust-lang/rust-bindgen/releases/download/v0.69.5/bindgen-cli-aarch64-apple-darwin.tar.xz"],

add on 2: I'm trying to build the cbindgen via rust_binary so I can use it to test for ffi.

add on 3: i have fixed the cbindgen issue in my local by build the cbindgen instead of download the release directly.

+
+load("@rules_rust//rust:defs.bzl", "rust_binary")
+
+
+rust_binary(
+    name = "cbindgen-cli",
+    srcs = [
+        "external/cbindgen_source/src/main.rs",
+        "external/cbindgen_source/build.rs",
+    ],
+    data = [
+        "external/cbindgen_source/Cargo.toml",
+        "external/cbindgen_source/Cargo.lock",
+    ],
+)
+
 # Generate the Rust binding file
 genrule(
     name = "iceoryx2-pal-posix-bindgen",
     srcs = [
         "src/c/posix.h",
-        "@bindgen//:bindgen-cli",
+        ":cbindgen-cli",
     ],
     outs = ["posix_generated.rs"],
-    cmd = "$(execpath @bindgen//:bindgen-cli) --use-core --blocklist-type max_align_t $(location src/c/posix.h) --output $(OUTS)",
+    cmd = "$(execpath :cbindgen-cli) --use-core --blocklist-type max_align_t $(location src/c/posix.h) --output $(OUTS)",
 )

However, it failed at some cpp issue, looks like it doesn't recognize the cpp17 feature. I will continue to figure it out.

external/iceoryx/iceoryx_platform/generic/include/iceoryx_platform/atomic.hpp:283:5: error: no template named 'enable_if_pointer_t'
    enable_if_pointer_t<U> fetch_sub(std::ptrdiff_t value, std::memory_order order = std::memory_order_seq_cst) noexcept

[xieyuschen]

Hi @elBoberido , i have change the bazel code and verify it could pass the test in my ubuntu so i think it's ready for you to review. The bazel doesn't work in my MacOS, but we can discuss it more in the issue so we can focus on the topic of this PR. thanks!

[elBoberido]

@xieyuschen it works great on bazel, even in external builds. On main there are some more cases that use the

#[doc(hidden)]
pub mod testing;

pattern. Would be great if you could also adjust those cases and also add an entry to the FAQ_ICEORYX_DEVS.md

[xieyuschen]

@xieyuschen it works great on bazel, even in external builds. On main there are some more cases that use the

#[doc(hidden)]
pub mod testing;

pattern. Would be great if you could also adjust those cases and also add an entry to the FAQ_ICEORYX_DEVS.md

Probably we can merge this first so you needn't take efforts to see this part of change again when i do the code change for the other cases. HDYT @elBoberido

Anyway, I will do the left things tomorrow.

[xieyuschen]

hi @elBoberido I have enabled the testing feature for test_suite only, so I believe there is no concern now:)

Please let me know whether you like this current approach, or we have a better way to do so. Then, after merging this PR and #487, I can support the others in this approach as well.

[xieyuschen]

hi @elBoberido , could you kindly review it when you have time? Thanks:)

[elBoberido]

@xieyuschen for now, I would just keep everything as is instead of adding a second rust_library. It's not only this one case but we use the doc hidden flag also in other places and this would result in quite a few lib duplications.

[xieyuschen]

hi @elBoberido , I think this is limited by bazel itself, we cannot enable a feature at the consumer side, ref.

In Bazel you can not select features of library in the consumer of the library, features should be defined in the library itself.

I don't like defining one more rust_library for testing as well, but that's the best way in bazel now. But given the case that the original goal is to hide test structures by adding new feature, looks like we don't have a consensus that whether this change makes things better.

Thanks for your review.

[elBoberido]

@xieyuschen I would suggest to keep this PR open and review the decision for the v0.6 development cycle.