ci: migrate Rust coverage from tarpaulin to cargo-llvm-cov#1279
ci: migrate Rust coverage from tarpaulin to cargo-llvm-cov#1279
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCI replaced cargo-tarpaulin with cargo llvm-cov via taiki-e/install-action@cargo-llvm-cov, added llvm-tools-preview to the Rust toolchain, updated coverage flags and ignore regex, removed tarpaulin.toml coverage config, consolidated engine test/release steps, updated artifact path to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Tool as Rust Toolchain (rustup / llvm-tools-preview)
participant Cov as cargo llvm-cov
participant Artifact as Actions Artifact
GH->>Tool: install toolchain + llvm-tools-preview
GH->>Cov: run `cargo llvm-cov --target-dir target --all-features -p iii --fail-under-lines 70 --ignore-filename-regex ...`
Cov-->>GH: coverage report + exit status
GH->>Artifact: upload `target/debug/iii` artifact
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
35-35: Consider pinning to a version tag for reproducibility.Using
taiki-e/install-action@cargo-llvm-covinstalls cargo-llvm-cov but doesn't pin the action version itself. For CI stability, consider pinning to a specific action version (e.g.,taiki-e/install-action@v2) and specifying the tool explicitly:- uses: taiki-e/install-action@v2 with: tool: cargo-llvm-covThis ensures consistent behavior across runs if the action's default behavior changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 35, Replace the unpinned action reference "uses: taiki-e/install-action@cargo-llvm-cov" with a pinned release tag and explicit tool selection to ensure reproducible CI runs; update the uses line to a specific version like "taiki-e/install-action@v2" and add a "with: tool: cargo-llvm-cov" entry so the action is pinned while still installing cargo-llvm-cov explicitly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 35: Replace the unpinned action reference "uses:
taiki-e/install-action@cargo-llvm-cov" with a pinned release tag and explicit
tool selection to ensure reproducible CI runs; update the uses line to a
specific version like "taiki-e/install-action@v2" and add a "with: tool:
cargo-llvm-cov" entry so the action is pinned while still installing
cargo-llvm-cov explicitly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fd8526cb-2f7d-4c70-b8ac-44c88e745c3d
📒 Files selected for processing (2)
.github/workflows/ci.ymltarpaulin.toml
💤 Files with no reviewable changes (1)
- tarpaulin.toml
guibeira
force-pushed
the
migrate-to-cargo-llvm-cov
branch
from
19aeb14 to
2ef2bcc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
44-45: Make the SDK exclusion regex robust to llvm-cov path formats.Line 45 uses
^sdk/, which matches only repo-relative filenames. Sincellvm-covapplies the regex to paths as recorded in the coverage mapping (which can be absolute or relative depending on compilation), absolute paths bypass this exclusion and cause coverage baseline drift. Use a pattern that matchessdk/regardless of whether it appears at the start or after a path separator.Suggested change
- "engine/src/main\.rs|(cron|pubsub|queue|state|stream)/adapters/(redis_adapter|bridge)\.rs|queue/adapters/rabbitmq/|^sdk/" + "engine/src/main\.rs|(cron|pubsub|queue|state|stream)/adapters/(redis_adapter|bridge)\.rs|queue/adapters/rabbitmq/|(^|.*/)sdk/"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 44 - 45, The ignore-filename-regex currently uses "^sdk/" which only matches repo-relative paths; update the regex inside the --ignore-filename-regex string (the value containing "engine/src/main\.rs|(cron|pubsub|queue|state|stream)/adapters/...|^sdk/") to match "sdk/" whether it appears at the start or after a path separator by replacing "^sdk/" with a pattern like "(?:^|/)sdk/" (ensure proper escaping/quoting consistent with the YAML string).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 37-56: Add an explicit build step to produce the standalone binary
before the artifact upload: insert a run step that executes cargo build -p iii
(optionally with --target-dir target or --release if you need release binary)
after the "Run coverage" step and before the actions/upload-artifact step named
"iii-binary" so target/debug/iii exists when uploaded.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 44-45: The ignore-filename-regex currently uses "^sdk/" which only
matches repo-relative paths; update the regex inside the --ignore-filename-regex
string (the value containing
"engine/src/main\.rs|(cron|pubsub|queue|state|stream)/adapters/...|^sdk/") to
match "sdk/" whether it appears at the start or after a path separator by
replacing "^sdk/" with a pattern like "(?:^|/)sdk/" (ensure proper
escaping/quoting consistent with the YAML string).
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
37-56:⚠️ Potential issue | 🔴 CriticalAdd an explicit build step before uploading the artifact.
cargo llvm-covruns tests viacargo testinternally and produces test harnesses undertarget/debug/deps/, not a standalone binary attarget/debug/iii. Without an explicitcargo build -p iiistep, the artifact upload will fail or upload nothing, breaking all downstream jobs that depend on this binary (sdk-node-ci, sdk-python-ci, sdk-rust-ci, motia-js-ci, motia-py-ci).Suggested fix
- name: Run coverage run: >- cargo llvm-cov --all-features -p iii --fail-under-lines 70 --ignore-filename-regex "engine/src/main\.rs|(cron|pubsub|queue|state|stream)/adapters/(redis_adapter|bridge)\.rs|queue/adapters/rabbitmq/|^sdk/" - name: Check formatting run: cargo fmt --all -- --check # - name: Run clippy # run: cargo clippy -p iii -p function-macros -p iii-sdk --all-targets --all-features -- -D warnings + - name: Build engine binary + run: cargo build -p iii + - uses: actions/upload-artifact@v4 with: name: iii-binary path: target/debug/iii retention-days: 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 37 - 56, The workflow is missing an explicit build step before uploading the iii artifact, so add a step that runs cargo build -p iii (or cargo build -p iii --release if you need release artifacts) after the "Run coverage" step and before the actions/upload-artifact@v4 step; ensure the new step has a descriptive name like "Build iii binary" and produces the target/debug/iii path referenced by the upload (or update the upload path if you build release).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 37-56: The workflow is missing an explicit build step before
uploading the iii artifact, so add a step that runs cargo build -p iii (or cargo
build -p iii --release if you need release artifacts) after the "Run coverage"
step and before the actions/upload-artifact@v4 step; ensure the new step has a
descriptive name like "Build iii binary" and produces the target/debug/iii path
referenced by the upload (or update the upload path if you build release).
2 participants
Summary
cargo-tarpaulinwithcargo-llvm-cov(LLVM source-based instrumentation — more accurate, faster CI)llvm-tools-previewrustup component (required by llvm-cov)taiki-e/install-action@cargo-llvm-cov(prebuilt binary, no compile-from-source overhead)tarpaulin.toml— exclusions now live as--ignore-filename-regexinci.ymlTest Plan
Build EngineCI job passes on this PRRun coveragestep outputSummary by CodeRabbit