Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run //:license-checkStatus: Click to expand output |
e370cc2 to
421bc08
Compare
|
The created documentation from the pull request is available at: docu-html |
pawelrutkaq
left a comment
There was a problem hiding this comment.
Fine until @dcalavrezo-qorix finishes miri integration for bazel.
- Disable flaky test. - Add pipeline with Miri.
421bc08 to
d4c0215
Compare
There was a problem hiding this comment.
Pull request overview
This PR enables running Rust tests under Miri in CI to catch undefined behavior early, while skipping a known-flaky test when executed under Miri (resolving #89).
Changes:
- Mark
unique_thread_runner_monitoring_worksas ignored when running under Miri. - Add a new GitHub Actions workflow to run
cargo miri teston pull requests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/health_monitoring_lib/rust/worker.rs |
Ignores one flaky test specifically under Miri. |
.github/workflows/miri.yml |
Adds a PR CI job that installs Miri and runs cargo miri test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
GITHUB_TOKEN is being exported as a job-wide environment variable, but this workflow doesn’t appear to use it directly. Consider removing it (or scoping it to the one step that needs it) to reduce unnecessary secret exposure in the runner environment.
| env: | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
|
|
||
| - name: Cargo Miri | ||
| run: | | ||
| cargo +nightly-2025-12-15 miri test --features stub_supervisor_api_client --no-default-features |
There was a problem hiding this comment.
The toolchain version is pinned twice (in dtolnay/rust-toolchain and in cargo +nightly-...). This duplication can drift over time; prefer relying on the installed toolchain (i.e., run cargo miri test without +...) or define the toolchain once (e.g., via an env var) and reuse it.
| cargo +nightly-2025-12-15 miri test --features stub_supervisor_api_client --no-default-features | |
| cargo miri test --features stub_supervisor_api_client --no-default-features |
| runs-on: ubuntu-latest | ||
| env: | ||
| MIRIFLAGS: "-Zmiri-ignore-leaks -Zmiri-disable-isolation" |
There was a problem hiding this comment.
-Zmiri-disable-isolation disables Miri’s isolation sandbox, which reduces the usefulness of CI Miri runs and broadens what tests can do on the runner. If it’s not strictly required, drop it; if it is required, add a brief note explaining what operation needs it so the flag doesn’t become a permanent blanket exception.
| runs-on: ubuntu-latest | |
| env: | |
| MIRIFLAGS: "-Zmiri-ignore-leaks -Zmiri-disable-isolation" | |
| runs-on: ubuntu-lest | |
| env: | |
| MIRIFLAGS: "-Zmiri-ignore-leaks" |
| } | ||
|
|
||
| #[test] | ||
| // Test is flaky for Miri. |
There was a problem hiding this comment.
The comment explains the test is flaky under Miri, but it doesn’t capture the failure mode or link to a tracking issue. Adding a short reason (e.g., timing/thread scheduling sensitivity) and/or an issue reference will make it clearer when it’s safe to re-enable the test for Miri.
| // Test is flaky for Miri. | |
| // Test is flaky under Miri due to timing and thread-scheduling sensitivity | |
| // in this sleep-based background thread check. |
Resolved #89